#207: the pause test asserts claiming stopped, not how fast a host is - #238
Merged
Merged
Conversation
Third build of the deployment image failed here:
FAILED tests/test_fleet.py::test_pausing_a_project_stops_claiming
AssertionError: work continued after the project was paused
assert 2 == 1
Nothing was wrong. The test paused a project, slept 100ms, sampled, slept
150ms and asserted the sample had not moved -- so an item already in flight
when the pause landed, finishing inside that window on a loaded machine, read
as "the pause did not work". The build host is a Docker-in-Docker worker under
contention, which is exactly the machine that exposes it.
#207 already named the fix: the property worth protecting is that pausing
stops *claiming*, and the old assertion measured how fast an in-flight item
finishes instead. So:
- `settle()` waits for completions to stop moving rather than assuming a
window, which tolerates a slow host instead of asserting against it;
- `queue.claim()` is called directly while paused and must return None. That
is the promise, tested with no timing assumption at all;
- the queue must still hold pending work, so a drained backlog can no longer
pass by proving nothing.
Ran six times on a contended machine without a flake. The resume half already
waited on a condition and is unchanged.
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
1 participant
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.
Third build of the deployment image failed on
test_pausing_a_project_stops_claiming_without_a_restart—work continued after the project was paused; assert 2 == 1.Nothing was actually wrong. The test paused a project, slept 100ms, sampled, slept 150ms, and asserted the sample had not moved. An item already in flight when the pause landed finishes inside that window on a loaded machine and reads as a broken pause. The build host is a Docker-in-Docker worker under contention — precisely the machine that exposes it.
#207 already named the fix: the property worth protecting is that pausing stops claiming; the old assertion measured how fast an in-flight item finishes.
settle()waits for completions to stop changing instead of assuming a window — it tolerates a slow host rather than asserting against one.queue.claim()is called directly while paused and must returnNone. That is the actual promise, with no timing assumption.Ran six times on a deliberately contended machine without a flake. The resume half already waited on a condition and is unchanged.
Failing run: https://github.com/TheDancingDeveloper-org/agent-harness/actions/runs/31246983747