fix(tests): make the resize waits one-directional - #29
Merged
Conversation
The stress gate caught this on 2.0.0's own change, at 16 threads, iteration 10 of 25 — which is what the gate is for. `resize_relayouts_the_frame` waited for `ready` before resizing, and `ready` is true of every frame. `wait_frame` returns the earliest frame nobody has looked at, so on a loaded runner that first call could hand back the frame the resize had already produced, leaving no later frame for the second wait and a ten-second timeout. termlens said as much: "the application has not completed a repaint since the frame this terminal last returned (2 complete frames in total)". Both waits are now one-directional, which is what makes the test deterministic rather than lucky. 2.0.0 gave the discriminator for free: the chosen line has room for its confidence interval at 110 columns and not at 80, so the interval's absence identifies a pre-resize frame and its presence a post-resize one. Neither predicate can be satisfied by a frame belonging to the other side of the resize. Audited the rest while here. Every other follow-up predicate is already one-directional — `ranking (`, `all refused configurations:`, `measured 11 of` and the scrolled row are each only true after their keypress — and the two bare `wait_frame(ready)` calls are the sole wait on a freshly spawned terminal. Resize was the only place two predicates could match the same frame. Twenty-five local iterations at 16 threads found no flake, which is not proof: the CI failure was at iteration 10. The stress matrix is. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.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.
The stress gate caught this on 2.0.0's own change —
hunt (ubuntu-latest, 16 threads), iteration 10 of 25. Which is what the gate is for.Cause
resize_relayouts_the_framewaited onreadybefore resizing, andreadyis true of every frame.wait_framereturns the earliest frame nobody has observed, so under load that first call could hand back the frame the resize had already produced — leaving no later frame for the second wait.Fix
Both waits are one-directional now. 2.0.0 supplied the discriminator for free: the chosen line has room for its confidence interval at 110 columns and not at 80, so the interval's absence identifies a pre-resize frame and its presence a post-resize one. Neither predicate can be satisfied by a frame from the other side of the resize.
Audited the rest
Every other follow-up predicate is already one-directional —
ranking (,all refused configurations:,measured 11 of, and the scrolled-row check are each true only after their keypress — and the two barewait_frame(ready)calls are the only wait on a freshly spawned terminal. Resize was the sole place two predicates could match the same frame class.Verification
25 local iterations at 16 threads, no flake — which is not proof, since the CI failure was at iteration 10. The stress matrix is, and I will run it before tagging.
Checklist