Repository navigation
Conversation
…CI race test_stale_retry_recovers_at_max_delay asserts on either side of the 3600s stale-retry threshold, but claim_execution compares against a fresh datetime.utcnow() internally rather than the `now` passed into it. Under load, any real delay between the test's setup and that internal check pushes the effective age across the boundary and flips the assertion — this is what flaked as test_stale_retry_recovers_at_max_delay[Alert-sqlite-3599-False] on master's Python-Unit CI. Freeze time for the test instead of relying on the real clock staying still. Co-Authored-By: Evan Rusackas <evan@rusackas.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Code Review Agent Run #935ce2Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #44586 +/- ##
==========================================
- Coverage 81.06% 81.06% -0.01%
==========================================
Files 2955 2955
Lines 178325 178325
Branches 41312 41312
==========================================
- Hits 144565 144563 -2
- Misses 31050 31052 +2
Partials 2710 2710
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This was fixed in #44651 so I'll be closing this PR off. If anything, this one has more comments :D |
SUMMARY
test_stale_retry_recovers_at_max_delayintermittently fails onmaster's Python-Unit CI, most recently astest_stale_retry_recovers_at_max_delay[Alert-sqlite-3599-False].The test asserts on either side of the 3600s stale-retry threshold:
age=3599should still be within the window (not eligible for reclaim),age=3601should be past it (eligible). It capturesnowonce and deriveslast_eval_dttmfrom it, butclaim_execution→_claim_execution(superset/commands/report/execution_claim.py:172) compares that against a freshdatetime.utcnow()call internally, not thenowthe test passed in. Sitting exactly 1 second inside the boundary, any real delay between the test's setup and that internal check — plausible under a loaded CI run withpytest -n auto— pushes the effective age past 3600s and flips the assertion.This is a test-only fix:
datetime.utcnow()inside_claim_executionis the correct production behavior (staleness has to be measured against real wall-clock time, not a caller-supplied value), so the fix freezes time for the test instead of changing what the code compares against.TESTING INSTRUCTIONS
All 4 non-skipped parametrizations pass locally, including the exact case that flaked in CI (
Alert-sqlite-3599-False).ADDITIONAL INFORMATION
🤖 Generated with Claude Code