Fix some flakey tests #851
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #851 +/- ##
=======================================
Coverage 98.02% 98.02%
=======================================
Files 444 444
Lines 36003 36016 +13
=======================================
+ Hits 35292 35305 +13
Misses 711 711
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 #851 +/- ##
=======================================
Coverage 98.02% 98.02%
=======================================
Files 444 444
Lines 36003 36016 +13
=======================================
+ Hits 35292 35305 +13
Misses 711 711
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 #851 +/- ##
=======================================
Coverage 98.02% 98.02%
=======================================
Files 444 444
Lines 36003 36016 +13
=======================================
+ Hits 35292 35305 +13
Misses 711 711
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 #851 +/- ##
=======================================
Coverage 98.02% 98.02%
=======================================
Files 444 444
Lines 36003 36016 +13
=======================================
+ Hits 35292 35305 +13
Misses 711 711
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| # assert that test_flag_bridges is a subset of tests | ||
| assert set(bridge.test_id for bridge in test_flag_bridges).issubset( | ||
| set(x.id for x in tests) | ||
| ) |
There was a problem hiding this comment.
previously this was testing for equality, not a subset relation. is this really what we want?
There was a problem hiding this comment.
Hmm, good point, the subset test doesn't really make sense. We might actually want to check that the test_ids in test_flag_bridge match the ones in test_instances (non-failure, I'm guessing). The flakiness with this test is that it assumes ordering of tests, where we should not have that assumption.
Since I'm having to guess the intent of this check, I can also be convince that this test isn't super useful, and just remove it. I'm leaning towards just modifying it to match the ids in test_instance out of conservatism.
This is fragile because hard-coding the pullid is more likely to run into conflicts
The first flush to create the commit wit pullid=12 actually creates it, but then it gets added again later. This just avoids flushing multiple times and attempts to create the pull first.
This avoids any other factories that might potentially use the pull object from recreating it in the database
Flushing the pull before creating the commit seems to reduce the chance of id collision. This is probably because commit _tries_ to create a pull. Flushing the pull logs the pull object in the DB, which disables the CommitFactory from also creating the pull object. This is my best understanding.
michelletran-codecov
force-pushed
the
fix_some_flakey_tests
branch
from
2982784 to
49fb6b4
Compare
michelletran-codecov
force-pushed
the
fix_some_flakey_tests
branch
from
49fb6b4 to
1adff7e
Compare
Extracted this from #729
A few of the tests become flaky when migrating to the Django models because of some unique references that's defined in Django, but not SQLAlchemy.
In particular, the way that FactoryBoy creates references can be flakey (i.e. Commits referencing Pulls. If the original
pullis not flushed, there is a chance that thecommitgets created first, which inadvertently creates a separate pull with theidof the one that you specified to create). So, doing an explicit flush will ofpullbefore creating thecommitwill fix this.Also fixed a test where DB returned results are not necessarily ordered.
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.