Refactored repository services tests to use fixtures #795
Conversation
This helps reduce database key conflicts by reusing existing objects for tests
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #795 +/- ##
==========================================
- Coverage 98.01% 98.00% -0.01%
==========================================
Files 443 443
Lines 36615 36566 -49
==========================================
- Hits 35887 35838 -49
Misses 728 728
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## main #795 +/- ##
==========================================
- Coverage 98.01% 98.00% -0.01%
==========================================
Files 443 443
Lines 36615 36566 -49
==========================================
- Hits 35887 35838 -49
Misses 728 728
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #795 +/- ##
==========================================
- Coverage 98.01% 98.00% -0.01%
==========================================
Files 443 443
Lines 36615 36566 -49
==========================================
- Hits 35887 35838 -49
Misses 728 728
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Ty for the whitespace suggestion @michelletran-codecov, it did make things easier to compare left <> right |
| ) | ||
| dbsession.add(commit) | ||
| dbsession.add(compared_to_commit) | ||
| dbsession.flush() | ||
|
|
||
| commit.pullid = "123" |
There was a problem hiding this comment.
This was extracted from line 1615. If I leave it in the factory create, it will try to link a pull object and was failing. Since we just needed it to be not null but not actually exist, it was just easier to set it outside the factory call. I don't know why it started failing after I switched to using Django to create the tables 🤷
| @@ -2076,6 +2031,10 @@ def test_fetch_commit_yaml_and_possibly_store_commit_yaml_invalid_commit_yaml( | |||
| mock_configuration.set_params({"site": {"comment": {"behavior": "new"}}}) | |||
| commit = CommitFactory.create( | |||
| repository__owner__yaml={"coverage": {"precision": 2}}, | |||
| # User needs to be less than PATCH_CENTRIC_DEFAULT_TIME_START | |||
There was a problem hiding this comment.
Ah, it's a remnant from #729 . Basically, the new Django models adds a default time of now (SQLAlchemy models will just have no default time for the owner object). If the owner is created after the 'PATCH_CENTRIC_DEFAULT_TIME_START', the default owner YAML is actually different and messed up with the tests. So, setting the timestamp is to preserve the test functionality of using the pre-patch_centric yaml behaviour. This doesn't really do anything bad right now, and will be useful later when I start creating the tables with Django later.
This helps reduce database key conflicts by reusing existing objects for tests
This refactor was pulled out of #729 to make changes more manageable. It solved a problem where recreating the objects had conflicts when I switched to Django models (duplicate key errors).
This is also significantly easier to review if you set "hide whitespace" to view the diff because I removed test classes.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.