test(runner): make the lease-expiry test observe the clock, not race it - #119
Merged
Conversation
`test_expired_unrenewed_lease_cannot_report_false_success` gave the lease an 80 ms life and slept 120 ms inside the run. The runner refuses a dispatch whose lease is already dead at the pre-start check, and bundle staging plus policy binding sit between the lease read and that check. When those took longer than 80 ms the job was refused before execution, `flow.calls` stayed empty, and the test failed with `assert 0 == 1`. It did exactly that on `main` for ubuntu-latest / Python 3.11. Drive the sequence by observation instead: - Patch `engine.runner_loop.datetime` with a real clock plus a test-controlled offset, and give the lease 30 s so the pre-start check always passes. - The patched run body waits for the renew loop's first extend attempt, then moves the clock past the deadline, then holds the run open until the renew loop stops attempting extends, which proves it observed the expiry. Every assertion is unchanged: the run executed once, at least one extend was attempted, the ack is `uncertain`, no `run_summary` reached the wire, and the journal records `uncertain`. The test now takes about 0.16 s instead of 0.12 s of fixed sleeps, and passed five consecutive local runs. Tests: full suite 951 passed, 6 skipped. Ruff passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
What
Remove the wall-clock race in
TestLeaseDiscipline::test_expired_unrenewed_lease_cannot_report_false_success(added in #116).
Why
mainfailed onubuntu-latest/ Python 3.11:The test gave the lease an 80 ms life and slept 120 ms inside the run. But
RunnerService._handle_jobstages the bundle and binds the effective policybetween reading the lease and the pre-start check
if datetime.now(timezone.utc) >= lease_deadline: raise Refusal(...). When thatwork took longer than 80 ms on a loaded runner the dispatch was refused before
execution, so
flow.runwas never called andlen(flow.calls) == 1failedwith 0.
This is a flaky test, not a product defect: refusing a dispatch whose lease is
already dead is the correct fail-closed behaviour.
How
Drive the sequence by observation instead of by guessed durations:
engine.runner_loop.datetimewith a real clock plus a test-controlledoffset, and give the lease 30 s so the pre-start check always passes.
moves the clock 60 s past the deadline, then holds the run open until the
renew loop stops attempting extends — which proves it observed the expiry.
Every assertion is unchanged: the run executed exactly once, at least one extend
was attempted, the ack outcome is
uncertain, norun_summaryreached thewire, and the journal records
uncertain. Nothing is skipped or relaxed.Tests
-k expired_unrenewed: 5 consecutive runs passed, ~0.16 s each.ruff check engine/ tests/ scripts/: passed.🤖 Generated with Claude Code