fix test that fails depending on run order #306
Conversation
If you run ``` pytest \ graphql_api/tests/test_impacted_file.py::TestImpactedFileFiltering::test_filtering_with_successful_flags \ compare/commands/compare/interactors/tests/test_fetch_impacted_files.py::FetchImpactedFilesTest::test_impacted_files_filtered_by_flags_and_commit_comparison_for_pull ``` you'll see the 2nd test fails. This is because of the PullID that apparently both tests create a pullID of 1. It is strange because they are `TransactionTestCase` and that's supposed to reset the DB, but God knowns what is going on. Hard setting the pullID to different numbers solves the issue, but I'll be the 1st to admit is not a pretty fix
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #306 +/- ##
=======================================
Coverage 95.95% 95.95%
=======================================
Files 611 611
Lines 15719 15719
=======================================
Hits 15083 15083
Misses 636 636
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report
@@ Coverage Diff @@
## main #306 +/- ##
=======================================
Coverage 95.95% 95.95%
=======================================
Files 611 611
Lines 15719 15719
=======================================
Hits 15083 15083
Misses 636 636
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -590,6 +590,7 @@ def test_impacted_files_filtered_by_flags_and_commit_comparison_for_pull( | |||
| CommitFactory(repository=repo), | |||
| ) | |||
| pull = PullFactory( | |||
| pullid=256, | |||
There was a problem hiding this comment.
- Does this mean setting to any ID less than 256 would not work?
- If you add a test (that creates a Pull object in the DB) that runs before this one, does that mean this would fail? Since the next available unused ID is 257?
There was a problem hiding this comment.
Does this mean setting to any ID less than 256 would not work?
No, it's just a random number.
If you add a test (that creates a Pull object in the DB) that runs before this one, ...
well the other test I mentioned that causes this issue creates another pull object in the database with ID 44. So, no?
Where did you get that the next available ID is 257?...
There was a problem hiding this comment.
Oh ok I didn't know it was a random number, I though anything below 256 is used.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #306 +/- ##
=======================================
- Coverage 95.57 95.56 -0.01
=======================================
Files 726 726
Lines 16228 16178 -50
=======================================
- Hits 15509 15459 -50
Misses 719 719
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
If you run
you'll see the 2nd test fails.
This is because of the PullID that apparently both tests create a pullID of 1.
It is strange because they are
TransactionTestCaseand that's supposed to reset the DB,but God knowns what is going on.
Hard setting the pullID to different numbers solves the issue, but I'll be the 1st
to admit is not a pretty fix