Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref: fix test pollution integrity errors due to cache leaks between tests #77476

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

asottile-sentry
Copy link
Member

this results in about a 15% failure rate:

$ pytest --count 10 tests/sentry/event_manager/test_event_manager_grouping.py -k test_records_hash_comparison_metric
...

failing with:

E   django.db.utils.IntegrityError: ForeignKeyViolation('insert or update on table "sentry_environmentproject" violates foreign key constraint "sentry_environmentpr_environment_id_f5d02227_fk_sentry_en"\nDETAIL:  Key (environment_id)=(18) is not present in table "sentry_environment".\n')
E   SQL: SET CONSTRAINTS ALL IMMEDIATE

I traced this down to a cache leaking from one test to another -- after applying this patch I had no failures in 100x runs of the tests

@asottile-sentry asottile-sentry requested a review from a team September 13, 2024 15:59
@asottile-sentry
Copy link
Member Author

also: the reason this is flaky is that sometimes the ids collide and sometimes they do not due to something non-deterministic in event classification -- haven't quite figured that part out yet

Copy link

codecov bot commented Sep 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #77476      +/-   ##
==========================================
- Coverage   78.11%   78.11%   -0.01%     
==========================================
  Files        6957     6957              
  Lines      308809   308809              
  Branches    50539    50539              
==========================================
- Hits       241231   241226       -5     
  Misses      61188    61188              
- Partials     6390     6395       +5     

@asottile-sentry asottile-sentry force-pushed the asottile-fix-integrity-errors-due-to-cache branch from 603dd22 to dba1b5e Compare September 13, 2024 17:49
@asottile-sentry asottile-sentry marked this pull request as ready for review September 13, 2024 18:00
@asottile-sentry asottile-sentry enabled auto-merge (squash) September 13, 2024 18:00
@asottile-sentry asottile-sentry merged commit f72a7fc into master Sep 13, 2024
47 checks passed
@asottile-sentry asottile-sentry deleted the asottile-fix-integrity-errors-due-to-cache branch September 13, 2024 19:40
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants