Skip to content

test(ProgressBar): wait for the story's bar, not 5 s - #10976

Merged
cixzhang merged 1 commit into
meta:mainfrom
vjeux:test/progressbar-story-wait
Nov 10, 2026
Merged

cixzhang merged 1 commit into
meta:mainfrom
vjeux:test/progressbar-story-wait

Conversation

@vjeux

@vjeux vjeux commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Maintainer impact

In the a11y suite under CI's profile (--cpus=4, CI=1, 4 contract workers), two ProgressBar tempo tests failed in the same second on main a0e0536. Each opened its story and asserted that the bar was visible within the assertion's 5 s default. Neither bar had rendered yet. Neighbouring tests took 36 and 42 s at that moment.

Intended invariant

The tests keep every claim and every bound. Only the wait for the story's first render changes: it ends on the bar itself, within the test's own timeout.

Change

d0a50f1 changes two spec files:

  • ProgressBarTouch's open() waits for its bar with waitFor(), not a 5 s visibility assertion.
  • ProgressBarMotion's open() and three tests that open the indeterminate story wait through one helper, barShown.
  • Storybook renders a story only after it has loaded the story's module. A loaded runner can take longer than 5 s for that. The harness's openStory waits for Storybook's render the same way, within the test's timeout. It has no globals, which these tests use for RTL and colour modes.

Evidence

  • Not-for-landing, CI's profile: the ProgressBar stories module held 6 s before it loads, in both tempo tests (5fa7d73 on main, ca752a1 on the fix).
    • On main 3582c71: 4 of 4 fail, expect(locator).toBeVisible() failed, element not found after 5 s.
    • At d0a50f1: 4 of 4 pass, in 11 to 13 s.
  • CI's profile, at d0a50f1: both spec files pass 52 of 52 (×2).
  • Product/runtime behavior verified unchanged: test-only.
  • CI impact: none.

Scope

  • 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

  • Static, at d0a50f1: typecheck, CI=true ESLint and Prettier on both specs, and review:pre pass.

Five ProgressBar a11y tests open a story, then assert that its bar
is visible within the assertion's 5 s default. Storybook renders the
story only after it has loaded the story's module. In the a11y suite
under CI's profile, a load spike held two of these tests past 5 s in
the same second, while neighbouring tests took 36 and 42 s.

The tests now wait for the bar itself, as the harness's openStory
waits for Storybook's render. The test's own timeout bounds the wait,
and nothing the tests judge changes.
@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 6:33am UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 10, 2026
@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, 6:43 AM UTC

@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · d0a50f1

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC Visual · A11y · RTL
ProgressBar · Storybook 11.7 kB (0) · 6.3 kB CSS (0) 568 (0) ✓ · ✓ · ✓

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

API changes: none in this pull request

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

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

Accessibility: 2 violation(s)

Accessibility Audit

Status: 2 accessibility violation(s) found — 2 serious.

ProgressBar - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 2/18 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Progressbar - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/1 stories · Learn more
    • WCAG: 1.4.3 (Level AA)

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.

Approved pending CI: CI has not run on this head yet. This is a draft from a fork, so once it is marked ready, a maintainer adds the ci:run label so CI runs.

The five story opens in the ProgressBar motion and touch specs now wait for the bar within the test's own 60 s timeout, and every assertion after them is unchanged. A story that loads slowly no longer fails at 5 s, and one that never renders still fails.

  • Optional: open() in ProgressBarMotion.a11y.chromium.spec.ts now waits on .first(), which drops the strict one-bar check the old assertion had; ProgressBarTouch's open() keeps it.

[Automated review]

* an assertion's 5 s for that. The test's timeout bounds the wait.
*/
async function barShown(page: Page) {
await page.getByRole('progressbar').first().waitFor();

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.

Optional: open() also waits through this helper, so its old strict getByRole('progressbar') check (exactly one bar in the Default story) becomes .first(). Touch's open() keeps the strict form. Consider letting open() call page.getByRole('progressbar').waitFor() and keeping .first() for the three indeterminate-story opens.

@cixzhang cixzhang added ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) ci:run-a11y Run the accessibility suite on this PR; write access, next push (AST-030 FR12) labels Nov 10, 2026
@vjeux
vjeux marked this pull request as ready for review November 10, 2026 19:04
@cixzhang
cixzhang merged commit 4236e88 into meta:main Nov 10, 2026
122 checks passed

This branch was successfully deployed

1 active deployment
Preview — d0a50f14 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) ci:run-a11y Run the accessibility suite on this PR; write access, next push (AST-030 FR12) 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