Repository navigation
ci: one docsite-test job builds the docsite once and runs the data tests and browser contracts on it - #8891
Conversation
…thout deepening history Checking out the PR head (#8812) made pr-scope-touches.sh deepen a shallow history back to the PR's recorded base. On long-lived branches that merge main often, the interaction-scope decide step went from a few seconds to up to three minutes, and one run hit the job's five-minute timeout, which fails the required test join. Both scope jobs now check out the merge commit with its parents and pass its first parent as the base, so the helper diffs the PR's own change against the base it merged into with no deepening. The merge queue keeps its base. A contract test pins the checkout and asserts no deepening on a branch that merged main four times.
…dates Pull request updates keep the fast checks and every required context. The visual, accessibility and RTL owners run for release-check, which Main now dispatches on each push to main (one at a time, skipping a commit main has moved past, failing Main when it fails), and which the release gate already runs before a cut. A pull request opts in with the ci:heavy-lanes label. A main failure is fixed forward. AST-030 FR12 and the new DEC-7 record the change.
One run per workflow and ref at a time, never cancelling a running check, so a burst of pushes is checked once at its newest commit, and Main's fast checks are no longer held behind the heavy wait.
lint.yml classifies a pull request's files with the trusted base classifier and skips full ESLint for a spec-only, docsite-only or tooling-only change. Its depth-1 checkout never let the merge commit reach its merge base, so the classifier was never fed and every pull request ran full lint (about four minutes of ESLint on a spec-only change). The checkout now takes the same 50-commit history ci.yml's check-scope does. A contract test runs the real step and classifier with real git.
The job checked out the whole history on a 4-core runner for every pull request into main, then learned from the branch name that only a release merge-back has anything to validate: about six minutes of larger-runner time per run for nothing. Its condition now names the merge-back branch pattern, so other pull requests skip it outright; the exact branch contract is still checked after the checkout, and required lint still validates merge-backs.
Internal Registry's dependency check runs only when a dependency declaration or the lockfile changes, the CLI smoke test only when a package, the workspace configuration or its own scripts change (its paths-ignore becomes an allowlist), and the hardening-issue opener only when Core component sources change. A registry records every workflow's kind and reason, and a contract test holds each workflow to it: no paths-ignore, no match-all glob, and no path filter on a workflow that reports a required context.
… classifier answered
…rds why (review) Internal Registry's header no longer says to make its check required: behind its path filter it never starts on a pull request that touches no dependency file. Re-enabling gating means dropping the filter and registering it as change-scope. A review-event workflow on pull_request_target now records in the registry why every pull-request update matters to review state, and the contract test requires it.
Removes the scope classifier jobs (check-scope, interaction-scope, native-swift-scope, check-components). Each job that owns tests now runs .github/actions/test-scope right after a sparse checkout: it reads the pull request's file list and the base commit's trusted classifier, and skips the job's remaining steps with success when nothing in its scope changed. Anything it cannot decide puts everything in scope (spec:AST-030 FR3, FR4). native-swift-runner is the one pre-job left, because only a job's runner has to be chosen before it starts. Also: - fixture-contrast's steps run in test-build (same Playwright runner); - registry-contract becomes test-registry-e2e and runs for release-check only; its network-free production gate runs with the unit tests; - ci-duration and the weekly a11y and RTL workflows are removed (the full audits run on main); - the Internal Registry job becomes test-deps, scoped by its trigger paths; - the test join requires release-bump on a release-branch pull request.
…sts and browser contracts on it docsite-data, docsite-browser and the docsite-test join become the one required docsite-test job (AST-030 FR7's name). It builds the packages and the docsite production build (which generates the data) once, then runs test-docsite (the data tests, in the background) and test-docsite-e2e (the Playwright contracts, its own named step) side by side on that build; a result step reports the data tests even after a contract failure. The spec-only lane still validates spec records here; the tooling lane skips. ci-docsite-browser.test.mjs pins the one-job shape, the step gates, the data-result step's exit code and the evidence upload; spec-owner-workflow and ci-tooling-routing follow the merged job.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
The main-tier release-check dispatcher runs on every landed commit, like the Main workflow, so it is a landed-commit workflow.
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: the single docsite-test job keeps the required context and builds the docsite once, but a failing docsite data test is not reported as written. GitHub runs these run: steps with bash -e, so the backgrounded subshell in test-docsite (start the data tests) exits before writing docsite-data.exit, and test-docsite (data test results) then polls until the 15-minute job timeout without printing docsite-data.log.
- Turn errexit off inside that subshell, for example
(set +e; pnpm -F @astryxdesign/docsite test > docsite-data.log 2>&1; echo $? > docsite-data.exit.tmp; mv docsite-data.exit.tmp docsite-data.exit) &, and consider bounding the wait loop. - In
ci-docsite-browser.test.mjs, run the start step's script underbash -ewith a failingpnpmstub and assert the start step'sid: dataand the result step's missingcontinue-on-error, since either regression currently lets a data-test failure pass with every test green. - Update the
CONTRIBUTING.mdsentence that still says adocsite-browserjob feedsdocsite-test.
[Automated review]
| - name: test-docsite (start the data tests) | ||
| id: data | ||
| if: steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' | ||
| run: (pnpm -F @astryxdesign/docsite test > docsite-data.log 2>&1; echo $? > docsite-data.exit) & |
There was a problem hiding this comment.
This subshell inherits the runner's bash -e (the workflow sets no shell:), so when pnpm -F @astryxdesign/docsite test fails it exits before echo $? > docsite-data.exit. The result step's while [ ! -f docsite-data.exit ] loop then runs until the job's 15-minute timeout, and docsite-data.log is never printed. Turning errexit off in the subshell, and writing the file atomically, fixes it:
(set +e; pnpm -F @astryxdesign/docsite test > docsite-data.log 2>&1; echo $? > docsite-data.exit.tmp; mv docsite-data.exit.tmp docsite-data.exit) &| it.each([ | ||
| [0, true], | ||
| [1, false], | ||
| ])('the data-test result step passes only on exit %i', (code, passes) => { | ||
| const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'docsite-data-')); | ||
| fs.writeFileSync(path.join(dir, 'docsite-data.log'), 'log\n'); | ||
| fs.writeFileSync(path.join(dir, 'docsite-data.exit'), `${code}\n`); | ||
| const result = spawnSync( | ||
| 'bash', | ||
| ['-e', '-c', step('test-docsite (data test results)').run], | ||
| {cwd: dir}, | ||
| ); | ||
| fs.rmSync(dir, {recursive: true, force: true}); | ||
| expect(result.status === 0).toBe(passes); |
There was a problem hiding this comment.
This runs only the result step against a pre-written exit file, so the start step's behaviour under bash -e is untested. Running step('test-docsite (start the data tests)').run with bash -e and a pnpm stub that exits 1, then expecting docsite-data.exit to hold 1, would catch it. Asserting that the start step's id is data and that the result step has no continue-on-error would pin the two other edges that currently let a data-test failure leave docsite-test green without failing any test.
The backgrounded subshell inherited the runner's errexit, so a failing data test run exited before writing docsite-data.exit and the result step waited for the job timeout. errexit is off inside the subshell and the exit file is written whole (tmp + mv). The contract test now runs the start step under bash -e with a pnpm stub (exit 1 fails on the previous head) and pins the start step's id and the result step's lack of continue-on-error. CONTRIBUTING.md names the docsite-test step instead of the removed job.
There was a problem hiding this comment.
Recommended for approval: docsite-test keeps the required context, builds the docsite once, and fails on either a data-test or a browser-contract failure, with the Vitest log printed by the result step. The backgrounded data-test step now records its exit code under bash -e, the contract test pins that path's fail-closed edges, and CONTRIBUTING.md names the new browser-contract step. CI has not run the merged job yet because this branch targets #8888's branch, so please check its wall time against the 15-minute limit on its first run against main.
[Automated review]
…per job Review follow-ups: - The registry contract, ci-duration, the weekly sweeps and the Internal Registry rename move to their own pull request, so this one only removes the scope classifier jobs and folds fixture-contrast into test-build. registry-contract stays a pull-request job here, deciding its own scope. - A ci:heavy-lanes pull request keeps its visual and RTL audits on the affected components (spec:AST-030 FR13): the scope action can also run the base's component-audit-scope.cjs and list the changed files, and the RTL and visual jobs read those instead of a classification job. - The interaction and native Swift scopes include the scope action, so a change to it runs the jobs it gates.
docsite-test takes main's pull-request gate: the required-context draft skip in its condition and the ci:run first step for outside contributors, and runs on ubuntu-latest like main's docsite build. docsite-data and docsite-browser stay removed, also from ci-duration's needs.
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: The accessibility audit did not run on this pull request. It runs after merges to Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
There was a problem hiding this comment.
Requesting changes for one small fix: on an outside contributor's pull request without ci:run, the always() evidence upload in docsite-test still runs after the hold step and fails with no files, so the required check shows a second, misleading error beside the ci:run reason. Gating that step on the scope step having run (for example always() && steps.scope.outcome == 'success' && …) and updating the matching assertion in ci-docsite-browser.test.mjs would resolve it. The rest of the merge keeps docsite-test required, fails it on either suite with its log, and has tests that catch each fail-closed edge.
[Automated review]
| exit "$(cat docsite-data.exit)" | ||
|
|
||
| - name: Upload docsite browser evidence | ||
| if: always() && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' |
There was a problem hiding this comment.
On an outside contributor's pull request without ci:run, the hold step fails and the scope step is skipped, so both scope outputs are empty and this always() condition is true: the upload runs with nothing checked out or tested and fails under if-no-files-found: error, adding a second error beside the ci:run reason. Gating it on the scope step having run keeps it after a build or contract failure:
if: always() && steps.scope.outcome == 'success' && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true'
The exact assertion at .github/scripts/ci-docsite-browser.test.mjs:144 would change with it.
On an outside contributor's pull request without ci:run, the hold step fails and the scope step never runs, so the always() evidence upload ran with nothing built and failed a second time on missing files. It now also requires the scope step to have succeeded, which keeps it after a build or contract failure.
There was a problem hiding this comment.
Recommended for approval: docsite-test keeps the required context, builds the docsite once, and fails on either a data-test or a browser-contract failure with its log. The evidence upload now skips when the ci:run hold stops an outside contributor's run, so that run shows only the hold reason, and the contract test pins the new condition. No changes are needed; because this branch targets another pull request's branch, CI has not run the merged job yet, so please check its wall time against the 15-minute limit once it runs against main. The Vercel preview deployment failed on this revision after passing on the previous one, and nothing it builds changed in between, so a re-run is worth trying.
[Automated review]
docsite-test takes over docsite-data's new production registry gate step (node internal/shadcn-registry/verify-production-gate.mjs), and its contract test reads it there. Main dropped ci-duration, so its needs list goes with it.
There was a problem hiding this comment.
Requesting one small test fix: docsite-test keeps the required context, builds the docsite once, passed on this revision in about 11 minutes, and fails on either suite with its log. When the hidden production registry gate moved here from docsite-data, it lost the exact-condition assertion that pinned it, so an extra skip on that step (for example skipping it when the scope step cannot decide) now leaves every .github/scripts test green, even though it is the only pull-request run of that gate for a docsite-only change. Adding 'Verify the hidden production registry gate' to the step list in ci-docsite-browser.test.mjs that already pins the active-lane condition closes it; the comment above test-registry-e2e in ci.yml also still says the gate runs in docsite-data.
[Automated review]
| 'Build docsite workspace dependencies', | ||
| 'Build the docsite', | ||
| 'test-docsite (start the data tests)', | ||
| 'test-docsite-e2e (browser contracts)', |
There was a problem hiding this comment.
Verify the hidden production registry gate (.github/workflows/ci.yml:458-460) is missing from this list, so nothing pins its condition any more: adding steps.scope.outputs.decided == 'true' && to it leaves every .github/scripts test green, although the base suite's exact-condition loop failed on that edit and the step is a docsite-only pull request's only run of the production gate (AST-030 FR9). Listing it here asserts the exact active-lane condition and no continue-on-error:
'test-docsite-e2e (browser contracts)',
'Verify the hidden production registry gate',The production registry gate step joins the list whose exact active-lane condition and lack of continue-on-error the contract asserts (AST-030 FR9); the test-registry-e2e comment names docsite-test.
There was a problem hiding this comment.
Recommended for approval: docsite-test keeps the required context, builds the docsite once, passed on this revision in under 10 minutes, and fails on either a data-test or a browser-contract failure with its log. The hidden production registry gate is back in the exact-condition list in ci-docsite-browser.test.mjs, so narrowing its condition now fails a test, and the test-registry-e2e comment names docsite-test. No changes are needed. One thing to keep in mind: the data tests' output now appears only after the browser contracts finish, so a run cut off by the 15-minute limit during the contracts shows none of it.
[Automated review]
Built on #8916–#8919 (#8888's split: each job decides its own scope), now on main.
Maintainer impact
A docsite-affecting pull request ran three jobs:
docsite-data(built core and the canary packages, generated the docsite data, ran its tests),docsite-browser(built core, the canary packages and every theme again, ran the docsite production build, which generates the data again, then the Playwright contracts) and thedocsite-testjoin. That is two package builds and two data generations per run, about 16,000 job-minutes a day, most of it on 4-core runners. Now one job builds once and runs both suites on that build in about the browser job's time, and the requireddocsite-testcontext reports straight from the work instead of from a join.Change
One
docsite-testjob (the required context, AST-030 FR7) replacesdocsite-data,docsite-browserand the join:spec:AST-030FR17): as a required context it keeps only the draft skip in its condition, and an outside contributor's run stops at its first step until a maintainer addsci:run. It runs onubuntu-latest, as main's docsite build does.verify-production-gate.mjs) after the data tests. The spec-only lane (and any run whose scope step could not decide) validates spec records here as before, the Node-tooling lane skips, and every other change builds and tests.test-docsite(the docsite data tests) runs in the background on that build whiletest-docsite-e2e(the Playwright browser contracts) drives it in its own named step. A later move of the e2e step to another tier can take it alone. A result step reports the data tests even after a contract failure, so neither suite can leave the context green when it failed.cancel-in-progressalready covers the job. Skipping drafts safely needsready_for_reviewin the workflow's triggers, a workflow-wide change, so it is not done per job here.Tests
ci-docsite-browser.test.mjsis rewritten for the one-job contract. It checks:docsite-dataanddocsite-browserare gone,docsite-testhas no needs, and there is exactly one docsite build;bash -ewith apnpmstub, records the data tests' exit code (0 and 1), with errexit off inside its background subshell and the exit file written whole;id, the result step's condition and its lack ofcontinue-on-error, that the result step waits while no exit code is recorded, and that it passes only on a zero exit;spec-owner-workflow.test.mjsandci-tooling-routing.test.mjsfollow the merged job, andCONTRIBUTING.mdnames thedocsite-teststep instead of the removed job..github/scriptssuite passes on this head (76 files, 1,422 tests).Lines
+187 / −230, net −43 against main c8e1804:
ci.yml+48/−121, tests +136/−106,CONTRIBUTING.md+3/−3.