Skip to content

test: three Switch and Tooltip marker reads decode their frames once - #11074

Merged
cixzhang merged 1 commit into
meta:mainfrom
vjeux:test/switch-tooltip-decode-once
Nov 10, 2026
Merged

cixzhang merged 1 commit into
meta:mainfrom
vjeux:test/switch-tooltip-decode-once

Conversation

@vjeux

@vjeux vjeux commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Maintainer impact

Three busy-proof reads decode one screencast more than once: once per control with trackControl, and once more for the marker with trackMarker.
On a loaded runner each decode pass over a cast takes seconds.
Since #10808, one trackMarkers call can read each control in its own strip.

Intended invariant

Each of these three reads decodes its frames once, however many colours it finds.
What each proof reads and asserts is unchanged.
P14's whole-cast marker read, the colour proof's brightness read and the toggle's reads still decode on their own.

Change

  • Switch's thumb clock, which two proofs share, reads both controls and the marker with one trackMarkers call.
  • Switch's track colour proof reads both controls with one call.
  • Tooltip's closed-trigger anchor proof reads the marker and the control with one call.

Evidence

  • Failure before the change: no timeout; the cost is time. On the same cast in Chromium, the separate reads took a median of 1.7 s, 0.6 s and 0.9 s.
  • Success after the change: one read took 1.0 s, 0.3 s and 0.7 s, and found the same tracks, frame for frame.
  • Product/runtime behavior verified unchanged: test only.
  • CI impact (for a change to workflows, actions or CI scripts): none.

Scope

Components: Switch, Tooltip (interaction specs only)

  • No intended public API, product behavior, visual direction, or policy change.
  • Product changes discovered during the work were removed or split.
  • Public text and artifacts contain no internal Meta context.

Testing

All figures are at the CI profile with one worker, on this head, 10f7314.

Same cast, median, the separate reads against one decode (d221877, not for landing):

Read Engine Runs Separate One decode
Switch thumb clock Chromium 8 1712 ms 1031 ms
Switch track colour Chromium 8 561 ms 305 ms
Switch track colour WebKit 8 1528 ms 782 ms
Tooltip anchor Chromium 4 858 ms 682 ms
Tooltip anchor WebKit 4 1718 ms 1201 ms

Each run times both forms on their own copies of the frames, in alternating order. One decode was faster in all 32 runs.

Identity commit, not for landing: 5603d7f.
Each converted read also runs the separate calls on copies of the frames and asserts equal tracks.
It passes 12 of 12 runs: 8 in Chromium and 4 in WebKit.

The four proofs at this head pass 12 of 12 runs in Chromium and 6 of 6 in WebKit.
The two Switch thumb-clock proofs run in Chromium only.

pnpm --filter storybook build
node_modules/.bin/playwright test --config playwright.interaction.config.ts --workers=1 \
  packages/core/src/Switch/__tests__/Switch.interaction.browser.spec.ts \
  packages/core/src/Tooltip/__tests__/TooltipClosedTriggerAnchor.interaction.browser.spec.ts

Prettier, ESLint (no new warnings), the core and harness type checks and review:pre are clean.

@vercel

vercel Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 10, 2026 7:47pm UTC

Request Review

@astracat-bot

astracat-bot Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 10, 2026, 7:51 PM UTC

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 10, 2026
@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · 10f7314

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC Visual · A11y · RTL
Switch · Storybook 104.7 kB (0) · 9.9 kB CSS (0) 1,734 (0) ✓ · ✓ · ✓
Tooltip · Storybook 30.4 kB (0) · 4.4 kB CSS (0) 870 (0) ✓ · ✓ · ✓

Package size: no published package changed; no new dependencies.

API changes: none in this pull request

Committed API report behind this build: cli 36 rows, core 36 rows; run pnpm api:report.

Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.


Generated by PR Enrichment workflow | View full report

@astracat-bot astracat-bot Bot 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.

Changes requested: the combined reads look right, and one wording fix is needed before approval.

  • The title and the stated invariant say Switch and Tooltip busy proofs decode each cast once, but three Switch busy proofs still decode frames twice (P14's whole-cast trackMarker read at Switch.interaction.browser.spec.ts:2578 before the thumb clock at :2605, the track colour proof's trackBrightness read at :3316, and the P10 toggle's sub-pixel and control reads at :2990 and :2998), so please retitle to the three reads this changes and scope the invariant to them.

Each combined trackMarkers call returns the same tracks the separate reads did, and the type checks, ESLint and Prettier are clean on the changed lines. CI has not run on this draft head yet.

[Automated review]

The Switch thumb's clock, the Switch track colour proof's control read
and the Tooltip anchor proof each read the controls with trackControl
and the marker with trackMarker, decoding one cast up to three times.
One trackMarkers call with controlColor now finds them in a single
decode per read. The tracks are unchanged.
@vjeux
vjeux force-pushed the test/switch-tooltip-decode-once branch from 7c6d474 to 10f7314 Compare November 10, 2026 19:34
@vjeux vjeux changed the title test: Switch and Tooltip busy proofs decode each cast once test: three Switch and Tooltip marker reads decode their frames once Nov 10, 2026

@astracat-bot astracat-bot Bot 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.

Approved pending CI: CI has not run on this head yet. It runs once the pull request is marked ready for review and a maintainer adds the ci:run label.

The new title and intended invariant match the change: each of the three converted reads now decodes its frames once, and the description names the reads that still decode on their own. Each combined trackMarkers call returns the same tracks as the separate trackControl and trackMarker calls it replaces, and the type checks, ESLint and Prettier are clean on this head.

[Automated review]

@cixzhang cixzhang added the ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) label Nov 10, 2026
@vjeux
vjeux marked this pull request as ready for review November 10, 2026 20:46
@cixzhang
cixzhang merged commit 8a8710c into meta:main Nov 10, 2026
165 of 171 checks passed
@vjeux
vjeux deleted the test/switch-tooltip-decode-once branch November 10, 2026 23:43

This branch was successfully deployed

1 active deployment
Preview — 10f73144 Deployed Nov 10, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants