Repository navigation
ci: one docsite-test job builds the docsite once and runs the data tests and browser contracts on it - #8891
ci: one docsite-test job builds the docsite once and runs the data tests and browser contracts on it#8891cixzhang wants to merge 23 commits into
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.
There was a problem hiding this comment.
Recommended for approval: merging #8888's branch keeps the one-job docsite-test valid by dropping the two deleted docsite jobs from ci-duration's needs, and data-test and browser-contract failures still fail the required check with their logs. No changes are needed; because this branch targets #8888's branch, CI has not run the merged job yet, so please check its wall time against the 15-minute limit on its first run against main.
[Automated review]
…ocsite-test validates Review follow-ups: - On a ci:heavy-lanes pull request, the accessibility, visual and RTL jobs decide their own component scope and skip their work, downloads included, when it does not apply, so a spec-, docsite- or tooling-only change no longer fails on a missing analysis artifact. The RTL join accepts shards that decided their scope and did nothing. - The scope action reports whether it decided. docsite-test validates spec records when its own scope is spec-only or undecided, so a failed lookup can never pass the required check unvalidated (spec:AST-030 FR4).
There was a problem hiding this comment.
Recommended for approval: after merging #8888's latest branch, the one docsite-test job still validates spec records on the spec-only lane and whenever its scope step cannot decide, and data-test and browser-contract failures still fail the required check with their logs. No changes are needed; because this branch targets #8888's branch, CI has not run the merged job yet, so please check its wall time against the 15-minute limit on its first run against main.
[Automated review]
There was a problem hiding this comment.
Changes requested: the single docsite-test job keeps the required context, builds the docsite once and fails on either suite with its log, but no test pins the result step's wait for the background data tests. With while [ ! -f docsite-data.exit ]; do sleep 2; done in .github/workflows/ci.yml replaced by [ -f docsite-data.exit ] || exit 0, the workflow suites in .github/scripts stay green, yet a failing data-test run that finishes after the browser contracts would then leave the check green, and AST-030 FR9 asks that removing any fail-closed edge make a test fail. Please add a test to ci-docsite-browser.test.mjs that starts test-docsite (data test results) with only docsite-data.log present, checks that it is still running, then writes 1 to docsite-data.exit (through a temporary file and a rename) and expects exit 1. Because this branch targets #8919's branch, CI has not run the merged job yet, so please check its wall time against the 15-minute limit on its first run against main.
[Automated review]
| it.each([ | ||
| [0, true], | ||
| [1, false], | ||
| ])('the data-test result step passes only on exit %i', (code, passes) => { |
There was a problem hiding this comment.
Both cases write docsite-data.exit before the step runs, so this pins the exit-code mapping but not the wait: with while [ ! -f docsite-data.exit ]; do sleep 2; done replaced by [ -f docsite-data.exit ] || exit 0, the workflow suites in .github/scripts stay green, while a data-test run that fails after the browser contracts would then leave docsite-test green (AST-030 FR9). A case that starts the step before the exit file exists would pin it:
it('the data-test result step waits for the data tests to finish', async () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'docsite-data-'));
fs.writeFileSync(path.join(dir, 'docsite-data.log'), 'log\n');
const child = spawn('bash', ['-e', '-c', step('test-docsite (data test results)').run], {cwd: dir});
const exited = new Promise(resolve => child.on('exit', resolve));
await new Promise(resolve => setTimeout(resolve, 500));
expect(child.exitCode).toBeNull();
fs.writeFileSync(path.join(dir, 'docsite-data.exit.tmp'), '1\n');
fs.renameSync(path.join(dir, 'docsite-data.exit.tmp'), path.join(dir, 'docsite-data.exit'));
expect(await exited).toBe(1);
fs.rmSync(dir, {recursive: true, force: true});
});… code A case starts the result step before docsite-data.exit exists: it must still be waiting 3 s later, then exit with the recorded code once the file appears. An early exit 0 in place of the wait now fails the suite (AST-030 FR9).
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, and ci-docsite-browser.test.mjs now fails if the result step stops waiting for the background data tests. No changes are needed. Because this branch targets #8919's branch, CI has not run the merged job yet, so please check its wall time against the 15-minute limit on its first run against main.
[Automated review]
Depends on #8919 (the last of #8888's split: each job decides its own scope); this PR targets its branch so the diff is only this change.
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: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
+163 / −217, net −54 against #8919 b85cca0:
ci.yml+40/−115, tests +120/−99,CONTRIBUTING.md+3/−3.