Accept zero-output as a valid seize in the recovery-disabled resample-merge test - #174
Merged
Conversation
On a loaded CI runner (observed on Windows), the resampler's warmup can lose the race with the chunk-15 seize: the reference jumps backward before the first merged message is ever emitted, the armed idle terminator fires after 4 s with an empty log, and last_t is -inf. That is the property under test -- output ceased and never reached the end of the stream -- so drop the n_msgs > 0 conjunct that turned it into a failure. Wiring liveness is already proven by test_resample_merge_healthy_system on the identical graph.
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.
Follow-up to #172, fixing the CI failure observed on PR #173 (Windows runner).
What happened
Exactly the failure mode anticipated in #172's description, converted from a silent multi-hour hang into a visible 4-second failure: under heavy runner load (both hubs logged
under subscriber backpressure), the resampler's warmup lost the race with the chunk-15 seize. The reference jumped backward before the first merged message was ever emitted, the armed idle terminator tore the graph down, and the test failed withn_msgs = 0,last_t = -inf.Why relaxing the assertion is the right fix (not later glitch_at / slower dispatch)
The property this test documents is: with recovery disabled, output ceases and never reaches the end of the stream. Zero output is that property — the seize simply won before the first emission. Retiming the schedule (
glitch_at/dispatch_dt) would only shrink the race window, leaving the flake in place for a sufficiently loaded runner.The
n_msgs > 0conjunct was a liveness sanity, but it is redundant:test_resample_merge_healthy_systemproves liveness of the identical wiring (no glitch) and asserts the stream reaches ~1.8 s. A regression that silenced the pipeline entirely would fail there.The seize upper bound (
last_t < 0.6) is unchanged, so a run where recovery does happen (output past the glitch) still fails.All 4 tests pass locally. After merge, re-running #173's checks should clear it — the flake is in dev's test, not that PR's code.