fix: let the killed-child supervisor test accept lease-loss retries - #49
Merged
Merged
Conversation
On a busy CI runner the second holder can miss a lease renewal with --lock-timeout 0.5, the reaper hands the task back, and a third claim runs it -- at-least-once execution doing its job: the stale run is refused (lost its lease) and the task still ends SUCCESSFUL. Assert what the worker promises instead of an exact count: attempts >= 2, one worker id per attempt, the first holder unchanged, and the lease-loss log line when a third attempt happens. Closes oxpull#47
Member
|
Thank you @dydydd ! great work looking forward to seeing you around again :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The exact
attempts == 2assertion breaks on busy CI runners: with--lock-timeout 0.5, the second holder can miss a lease renewal, the reaper hands the task back, and a third claim runs it — at-least-once semantics doing their job (stale result refused via "lost its lease", task ends SUCCESSFUL).Asserts what the worker promises instead of an exact count:
attempts >= 2, one worker id per attempt, first holder unchanged, andlost its leasein the log when a third attempt happens. Kept the SIGKILL-restart and final-status checks untouched, per #47.Verified: targeted test + full
tests/test_supervisor.py(25 passed) on Python 3.13 / SQLite;ruff checkandruff format --checkclean. Full suite: 1066 passed, 1 pre-existing failure intest_import_beat(zoneinfo translation) that reproduces on a clean checkout here, unrelated to this change.Closes #47