Skip to content

animations: allow WebKit's microsecond duration rounding in the confetti timing test - #345220

Closed
Bryan Chen (bryanchen-d) wants to merge 1 commit into
mainfrom
copilot/vscode/confetti-timing-precision
Closed

Bryan Chen (bryanchen-d) wants to merge 1 commit into
mainfrom
copilot/vscode/confetti-timing-precision

Conversation

@bryanchen-d

Copy link
Copy Markdown
Collaborator

Main CI 491287 failed on macOS ARM64 Run unit tests (Browser, Webkit) with Animations › creates consecutive pixelated rainbow confetti animations, where allParticlesUseBalancedTiming was false. The same commit passed in the previous main CI.

Cause

triggerConfettiAnimation computes each keyframe offset as time / duration, using the full-precision random duration (for example 1490.6732…). The test multiplies the offset step by effect.getTiming().duration and requires the result to be <= 40 + 1e-10. WebKit reports that duration rounded to whole microseconds (1490.673). Whenever the rounding goes up, each 40 ms step reads as up to about 1e-5 ms longer, which fails the 1e-10 tolerance. The duration is random, so the test fails about half the time on WebKit. Chromium and Electron report the exact value.

I confirmed this in local WebKit with a temporary probe over 40 animations. Every failing case had d with exactly 3 decimals and maxGap − 40 between 5e-6 and 1.3e-5, while easing, keyframe counts and duration bounds were all fine.

Change (test only)

The tolerance goes from 1e-10 to 1e-3 ms, which covers microsecond rounding, and a comment says why. That's still 4 orders of magnitude tighter than anything visible, so the test still checks that the steps are 40 ms. The product is unchanged.

Validation (local, --run animations.test.ts)

  • Before, WebKit: 5 of 10 runs failed.
  • After:
    • WebKit: 0 of 20 runs failed.
    • Chromium: 0 of 20 runs failed.
    • Electron: 9 passing.
  • eslint is clean.

Authored with Copilot

Copilot AI balanced review requested due to automatic review settings October 30, 2026 16:56
@bryanchen-d

Copy link
Copy Markdown
Collaborator Author

Lee Murray (@mrleemurray) this is a test-only follow-up to #345182. The confetti timing check failed on main CI's WebKit job (491287). WebKit reports getTiming().duration rounded to whole microseconds, but the keyframe offsets come from the unrounded random duration, so a 40 ms step can read as about 1e-5 ms over the test's 1e-10 tolerance. That happens about half the time. This PR loosens the tolerance to 1e-3 ms. The local probe and before/after runs are in the description.

Co-authored with Copilot

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues were identified for the test-only adjustment accommodating WebKit duration rounding.

No open issues 🎉

What changed in this PR

This test-only change addresses intermittent WebKit failures in the confetti animation timing test by allowing for microsecond rounding of reported durations. Production animation behavior is unchanged.

Changes:

  • Increases the timing tolerance from 1e-10 to 1e-3 ms while retaining the existing timing checks.
  • Adds a comment explaining why WebKit needs the tolerance.
File Description
src/​vs/​base/​test/​browser/​ui/​animations/​animations.test.ts Adjusts the confetti timing assertion tolerance and documents WebKit’s duration rounding.

Review effort: Balanced

@mrleemurray

Copy link
Copy Markdown
Contributor

Bryan Chen (@bryanchen-d) gotcha, thanks for updating this!

…tti timing test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues were identified for the test-only adjustment accommodating WebKit duration rounding.

No open issues 🎉

Review effort: Balanced

@bryanchen-d

Copy link
Copy Markdown
Collaborator Author

Closing this as a duplicate of #345214, which merged the same 1e-3 tolerance change to animations.test.ts. Thanks Lee Murray (@mrleemurray) for reviewing.

Co-authored with Copilot

@bryanchen-d
Bryan Chen (bryanchen-d) deleted the copilot/vscode/confetti-timing-precision branch November 4, 2026 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants