test(e2e): read recorded animations instead of racing getAnimations() - #169
Merged
Merged
Conversation
`tests/e2e/animation.spec.ts` inspected FLIP animations by calling
`getAnimations()` after `page.dragAndDrop()` returned. That list only holds
animations still running, so the read raced the 150ms animation it was
inspecting: on a loaded runner the FLIP finished before the `page.evaluate`
round-trip landed and the call came back empty.
Because the race is load-dependent, all three Playwright retries lose
together on a slow machine, so it surfaces as a hard failure rather than an
obvious flake. Seen on WebKit in run 33324767753:
Error: expect(received).not.toBeNull()
Received: null
at tests/e2e/animation.spec.ts:145:22
A longer timeout cannot fix this. The observation window opens when the
animation starts and closes when it ends, and waiting only moves the read
further past the close.
`recordAnimations` patches `Element.prototype.animate` through
`addInitScript` so every `Animation` is stored on the element that created
it. `flipInfo` and `animatingCount` now read that record, which outlives the
animation, so the assertions no longer depend on when the read lands.
That also removes the reason for the 500ms widening in 'should animate items
affected by reordering'. It was there to hold the window open long enough to
observe, and the test asserts which items animate, not for how long. The
test now runs at the default duration.
Verified: all 8 tests in the file pass on Chromium, and still pass when a
full `waitForAnimations` is forced ahead of every read — the worst case for
the old approach.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JktBrqK6LaxcbiiTfDyUC7
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
🎉 This PR is included in version 3.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
The failure
tests/e2e/animation.spec.tsblocked PR #166 — release retry and 3.0.0 notes on WebKit in run 33324767753:PR #166 changes only
release.ymlandCHANGELOG.md. The same test passed on two other PRs branched from the same commit, and passed on re-run.The cause
flipInfoandanimatingCountreadgetAnimations()afterpage.dragAndDrop()returned. That list only holds animations that are still running, so the read raced the 150ms animation it was inspecting — on a loaded runner the FLIP finished before thepage.evaluateround-trip landed, and the call came back empty.Because the race is load-dependent, all three Playwright retries lose together on a slow machine, so it presents as a hard failure rather than an obvious flake.
A longer timeout cannot fix it. The observation window opens when the animation starts and closes when it ends; waiting only moves the read further past the close.
The change
recordAnimationspatchesElement.prototype.animateviaaddInitScript, so everyAnimationis stored on the element that created it.flipInfoandanimatingCountread that record, which outlives the animation. The reads no longer depend on when they land.That also removes the reason for the 500ms widening in
should animate items affected by reordering— it existed only to hold the window open, and the test asserts which items animate, not for how long. It now runs at the default duration.Verified
Forcing a full
waitForAnimationsahead of every read reproduces the worst case deterministically — the animations are guaranteed finished before the assertion:Received: null,Received: undefined,Received: 0Both runs on Chromium. Unmodified, the file also passes 8/8 after the change.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JktBrqK6LaxcbiiTfDyUC7
🔖 Claude agent: M2:Resortable Release
session id:
4abbd3dd-78c3-4334-a069-05481c380a2f