test(island): the pass's aim is read once the band stands at it - #1148
Merged
Conversation
`aimedAt` took the first inline height it found on the swipe element and called that the figure the pass is aiming at. `carry()` in `BandSwipe.vue` pins the height it is leaving before it can measure the one it is arriving at, so on a stop arriving without a gesture the first height there is where the band was rather than where it is going. The helper now takes the height stood at and waits past it, which tells the two apart. Measured on the failing read: from=556 swiped=914 hit=914 stands=681, where `hit` should have been 681 and instead repeated the height the swipe had left behind. The race is in the helper rather than in the band, and it has always been there: which of the two heights is read depends on when the poll lands between the pin and the measure.
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.
Why
aimedAtreads the height a pass is aiming at off the band's inline style, and takes the firstnon-null figure it sees. That figure is not always the one the pass is aiming at.
carry()inBandSwipe.vuepins the height it is leaving a tick before it can measure the oneit is arriving at, so for a moment the inline height is the departing stop's. A stop arriving
without a gesture is read in exactly that moment.
What this achieves
The helper reads the figure the pass aims at, rather than whichever figure happens to be pinned
when it looks. Measured on the board page:
from=556 swiped=914 hit=914 stands=681— thathitshould have been 681, and the assertion comparing them passed only because both were wrong the
same way.
How
The reader is told the height the band already stood at, and waits past it. Deterministic: no
timeout raised, no bare wait, and the assertions themselves are untouched.
Not in scope
The Playwright upgrade for #852, which is the pull request that follows this one and needs this
one merged first — without it, the upgraded Chromium reads the departing height and the case
fails with
Received: 0.Worth a reviewer's attention
This is a pre-existing race, not something a newer browser introduced. It reproduces on the
Playwright this repository pins today: the case it fixes was green in 5 of 5 full-file runs on
1.60.0, and the fix holds there too, so this stands on its own rather than as preparation.
Verification
yarn typecheck,yarn lintandyarn test:unit(1540 tests over 179 files) clean, all exit 0.boards-swipe.motion.spec.tsin the motion project, five full-file runs on the currentPlaywright: 9/9, 8/9, 9/9, 9/9, 9/9. The single failure was
travels the line to a board a finger arrived at, which this change does not touch and which never callsaimedAt— it passed 6 of 6when re-run alone, and reproduces on unmodified main. The known starvation flake, 8 workers on 4
vCPUs.
Diff breakdown —
█added░removed, scaled to the largest row.