Repository navigation
ci: each accessibility leg downloads the Storybook before it counts - #10453
Conversation
A pull request's selection resolves its components through the built Storybook's story index, so the count step failed on a missing index.json in every leg. The download now comes before the count in every leg.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
cixzhang
added
the
ci:run-a11y
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Before | After | Change | |
|---|---|---|---|
| Job-minutes per PR update | 112.4 | 112.4 | 0 |
| Runner pool, runner-minutes a day | 58,103 | 58,114 | +10 (0%) |
| Average concurrent jobs | 40.3 | 40.4 | 0 |
| Time to green (median → this run) | 17.9 min | 18.3 min | +0.5 min |
| Runs a day | 517 | 517 | 0 |
Changed jobs: pr-a11y changed: standard or non-standard by event (2-core-ubuntu-arm)
Not measured (the jobs that use these keep their recent minutes above): .github/scripts/a11y-pr-contracts.mjs, .github/scripts/a11y-pr-legs.test.mjs.
Assumptions: before is each job's average over the last 20 pull-request runs; after keeps that for unchanged jobs and uses this run's minutes for added or changed ones, weighted by how often each changed job ran in those runs; runs a day averages the seven whole days before today; time to green compares this run with the median of those runs that ran the same jobs.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: moving the Storybook download ahead of the count is the right fix, and the contract test fails against the old step order as it should. One thing needs fixing before this lands:
- The description and commit message say every
ci:run-a11ypull request's count step failed, but the count opens the story index only to expand a changed folder that holds several public components (.github/scripts/a11y-pr-scope.mjs:166-170), so pull requests that change only single-component or shared folders already counted and ran their specs; please narrow those lines to that case (43 folders today, among them coreDialog,DropdownMenu,Table,AvatarandChat).
The folder list in my earlier review (core Chat, Indicator, Layer, NavMenu, Resizable; Lab Chart, Chat, Radial, Sankey, ThreeD) was incomplete, so please don't copy it. Optionally, the new comments in ci.yml, a11y-pr-contracts.mjs and a11y-pr-legs.test.mjs can take the same qualifier.
[Automated review]
Only a change to a multi-component folder resolves its components through the story index; the comments say so.
There was a problem hiding this comment.
Approving: moving the Storybook download ahead of the count restores the opted-in accessibility run for pull requests that change a multi-component folder, the order test fails against the old step order as it should, and the description now names the affected case.
- The required
testcheck is red becausetest-node (3/4)failsci-interaction-webkit.test.mjs("runs only the specs whose code its scope covers"), which now also findsCarouselSnapPagingEdges.interaction.browser.spec.ts; that spec landed onmainafter this branch's base, so the failure comes frommain, not this change, and the merge waits for its fix. - Optional: the description's list ("That covers core's Chat, Indicator, Layer, NavMenu and Resizable, and Lab's Chart, Chat, Radial, Sankey and ThreeD") names 10 of the 43 folders that resolve through the story index;
Dialog,DropdownMenu,Table,Avatarand others also hold more than one public component, so "43 folders today, among them …" would be accurate.
[Automated review]
…list Main gained a second Carousel interaction spec, which the fixed one-item list rejected.
There was a problem hiding this comment.
Approved pending CI: CI has not finished on this head yet. Moving the Storybook download ahead of the count restores the opted-in accessibility run for pull requests that change a multi-component folder, and the WebKit Carousel case now reads the folder's interaction specs, so it passes again on a merge with main, which added a second Carousel spec.
- Optional, description: say why the WebKit routing test changes, widen the folder list to "43 folders today, among them …" (
Dialog,DropdownMenuandTablealso resolve through the story index), and copy the PR report's CI-impact figures (112.4 to 112.5 job-minutes per update, +27 runner-minutes a day). - #10465 and #10469 make the same WebKit fix on the same lines with a fixed two-item list, so whichever of the three lands second needs a conflict resolution; this version needs no further edit when another Carousel spec is added.
[Automated review]
Main already fixed the WebKit Carousel case; this keeps main's version of that test.
There was a problem hiding this comment.
Approved pending CI: this push deferred the heavy suites, so test, build and docsite-test report the deferral and run when all jobs are re-run before merge; lint and typecheck passed. Moving the Storybook download ahead of the count restores the opted-in accessibility run for pull requests that change a multi-component folder, and the order test fails against the old step order as it should. With main merged in, the WebKit Carousel test now comes from main's own fix, so this pull request no longer changes it.
- Optional, description: widen the folder list to "43 folders today, among them …" (
Dialog,DropdownMenuandTablealso resolve through the story index), and copy the PR report's CI-impact figures (112.4 job-minutes per update before and after, +12 runner-minutes a day).
[Automated review]
Who this helps
A pull request opted into accessibility specs (
ci:run-a11y) that changes a multi-component folder gets its specs again. That covers core's Chat, Indicator, Layer, NavMenu and Resizable, and Lab's Chart, Chat, Radial, Sankey and ThreeD. Since the browser specs were split into legs, each leg's count step failed within a minute on such a change:ENOENT … apps/storybook/dist/index.json. One pull request had all three legs fail that way, with no spec run. A pull request that changes only single-component folders was not affected.Why
To find a changed multi-component folder's components, a pull request's selection (
a11y-pr-scope.mjsover the component analysis) reads the built Storybook's story index. The count step selects, but the Storybook artifact was downloaded only after it, and only on a leg with a share. The count's own test used a dispatch, which resolves no components.What changes
.github/workflows/ci.yml,pr-a11y-specs: the Storybook download moves before the count step and runs in every leg that has components, as the count does.CI impact
ubuntu-24.04-arm, as today. No new job or required context.Tests
a11y-pr-legs.test.mjs: the Storybook download comes before the count under the count's own condition, and only the install, run and upload wait for a share.ci:run-a11y, all three legs counted and passed.main's workflow.