Uh oh!
There was an error while loading. Please reload this page.
test(ci): a dist vitest project so built-artifact claims can be pinned - #7291
Merged
Conversation
The root `vitest.config.mts` aliases every workspace package to its `src`, which is right for the ~2000 tests that want fast source feedback and leaves "does the SHIPPED BUNDLE still do X" unanswerable. Such a test could not be committed: turbo's `test` task is `dependsOn: ["^build"]` — the DEPENDENCIES' builds, not the package's own — so a `dist`-importing test landed in CI with no `dist` to import. That is NOT MEASURED rather than a red pin, and the usual repair (delete it, or let it skip when `dist` is missing) leaves a green suite that measures nothing. Two lanes hit this wall in one morning and each threw a correct measurement away. Implements the PM ruling on objectui#7183 (option 1, 2026-09-02): - `vitest.config.mts` gains a fourth project, `dist`, collecting `packages/*/src/**/*.dist.spec.tsx`. The suffix keeps these files out of `unit`, `dom` and `dom-heavy` BY CONSTRUCTION — none of the three needed a character changed — and out of each package's `tsconfig.test.json`, which matters because turbo's `type-check` waits on `^build` and must never read the package's own `dist` (objectui#4801). - The project is OPT-IN behind `OBJECTUI_DIST_PINS=1`. CI's test job runs `pnpm test` with no build step, so an unconditional fourth project would be collected there with no `dist` on disk and would fail every PR. The one false-green this opens — `--project dist` without the env var collecting zero files and exiting green — is refused in the config with a message naming the right command. - `turbo.json` gains `test:dist`, `dependsOn: ["build"]` (self, not `^build`), `cache: false`: a lane whose subject is a build artifact turbo does not hash must not replay a verdict. That places it on the uncached side of the partition `scripts/__tests__/turbo-task-guard-coverage.test.ts` enforces, whose docstring is updated to match. - The light dom setup is deliberate, not a cost optimisation: `vitest.setup.dom.tsx` registers `page:header` from SOURCE, which would keep a pin green with the built bundle removed entirely. First resident: the objectui#6252 acceptance criterion PR objectui#7180 measured by hand and could not commit — an id-authored `page:header` resolves through the BUILT renderer and carries no `body.source` into the DOM or the authored node. Its live control is part of the pin: with the `dist` import removed the run fails `expected undefined to be truthy`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Sep 2, 2026
yinlianghui
commented
Sep 2, 2026
CollaboratorAuthor
CI red on
Generated by Claude Code |
… still type-checks The `dist` project entry added for objectui#7183 sits inside a conditional spread, and inside that array literal `extends: true` widens to `boolean`. `TestProjectConfiguration.extends` is `string | true | undefined`, so the widened element matches no `defineConfig` overload and the whole `projects` array degrades to `never[]`: ../../vitest.config.mts(311,7): error TS2769: No overload matches this call. ../../vitest.config.mts(325,7): error TS2769: No overload matches this call. vitest.config.ts(11,15): error TS2345: Argument of type 'UserConfig & …' is not assignable to parameter of type 'never'. CI reported it on `@object-ui/console#type-check` rather than here, because `apps/console/vitest.config.ts` merges this config and is the type program that reads it — the root `.mts` is compiled by nobody on its own. `as const` keeps the literal narrow. The three pre-existing projects are unaffected: their `extends: true` sits in the plain array, where it never widened. A comment records why the annotation is load-bearing, since it reads like removable noise. Measured red -> green on the reported program, deps built through turbo first (the errors are unreachable behind 378 TS2882s on an unbuilt tree): before exit 1, 3 errors after exit 0, 0 errors Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
September 2, 2026 05:14
This was referenced Sep 2, 2026
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#7183
Implements the PM ruling on this card (2026-09-02, option 1): a dedicated
distvitest project with its own turbo task that builds the package under test first.The gap
The root
vitest.config.mtsaliases every workspace package to itssrc. That is right for the ~2000 tests that want fast source feedback, and it makes "does the SHIPPED BUNDLE still do X" structurally unanswerable. Such a test could not be committed either: turbo'stesttask isdependsOn: ["^build"]— the DEPENDENCIES' builds, never the package's own — so adist-importing test landed in CI with nodistto import. That is NOT MEASURED rather than a red pin, and the usual repair (delete it, or let it skip whendistis missing) leaves a green suite that measures nothing. Two lanes hit this wall in one morning and each threw a correct measurement away.The five binding constraints, and where each is met
1. Only built-artifact pins; first resident re-derived from PR #7180; no source-resolved test moves in.
The project collects exactly one glob,
packages/*/src/**/*.dist.spec.tsx(spelled in the config, not here — a star-slash pair inside a block comment ends the comment, which this PR learned the hard way, see Corrections below). Its only resident ispackages/components/src/__tests__/page-header-action-ids.dist.spec.tsx: an id-authoredpage:headerresolves an action whose definition carries a script body, and the marker reaches neither the rendered DOM nor the authored node. Nothing was moved into the project; no existing test changed.2.
dependsOn: ["build"]for the package under test (self, not^build), and a loud precondition — not a skip — whendistis absent.turbo.jsongainstest:distwithdependsOn: ["build"]andcache: false. The precondition is an explicit assertion that reads the built entry from the package's ownpackage.jsonexports["."].importand names the absolute path when it is missing. It fails; it does not skip.3. The live control from PR #7180 is part of the delivery.
registers page:header from the BUILT bundleis a baretoBeTruthy()on purpose — a custom message would change the string the control is recorded by. See verdict (iii).4. Wired where the existing test job runs; no timeout raised, no heavy-test allowlist entry.
One step in the existing
testjob of.github/workflows/ci.yml,if: … && matrix.shard == 1.timeout-minutes: 20is untouched,heavyDomTestsis untouched, and the three existing projects (unit,dom,dom-heavy) are untouched.5. The step-2 tripwire. Not tripped, but it was close, and the near-miss is the design: see The opt-in below.
The opt-in — the half that keeps
pnpm testunchangedCI's
testjob runspnpm test(vitest run) with no build step anywhere in it. An unconditional fourth project is therefore collected by that run with nodiston disk, and its precondition would fail the whole suite on every PR — which would have forced a build into the path of all ~2000 tests, i.e. exactly the "change to how all tests build" constraint 5 says to stop at.So the project is declared only when
OBJECTUI_DIST_PINS=1. An env var rather than the--project distflag, because argv is meaningful only in the process that parsed the CLI while the env var is inherited by everything Vitest spawns.That opens exactly one false-green, and it is closed in the config rather than documented:
vitest run --project distwithout the env var would match no project, andpassWithNoTestsis true for a run that names no files — a green that measured nothing. Measured:The three verdicts
Each quotes the run's own output. Both mutating legs restore under a
trapwith absolute paths and prove the mutation landed on disk by marker count — never by an editor's exit code.(i)
distpresent, run the way CI runs it — GREEN.(ii)
distdeleted — a loud, named precondition FAILURE, not a skip and not MODULE_NOT_FOUND.Restore proven by state, not by an exit code:
sha256of the restoreddist/index.jsequals the recorded5365d829….(iii) the
distimport removed — the recorded control.expected undefined to be truthy— verbatim what PR #7180 recorded. The one case that still passes is the precondition, which is correct:distis on disk in this leg; only the import is gone. Restore proven by state: restored blob1f531adf…equals the HEAD blob, andgit diff HEADandgit status --shortare both empty.Cost — measured, because the ruling priced this at "one build in one job"
A bare
turbo run test:distdoes not cost one build. Turbo applies a task definition to every package in scope and resolvesdependsOn: ["build"]for each, so it schedules a build of the entire monorepo —siteandconsoleincluded:The root script therefore carries
--filter=@object-ui/components, and the 9 tasks are the package plus the dependency closure its.d.tsemit genuinely needs. Cold-cache wall time, which is what CI pays:~72 s added to shard 1 of a job whose ceiling is
timeout-minutes: 20, unchanged. (Shared-box seconds: four sibling agents build in this container, so this is an upper-ish bound, not an idle-box figure.)If a second package ever gains a pin without being added to the filter, the failure is the loud precondition naming its missing built entry — not a silent skip.
Collected by exactly one project
vitest list --filesOnlyis the instrument, and it has a positive control:The suffix does this by construction —
unitcollects*.test.ts,domcollects*.test.tsx,dom-heavyis an explicit file list — so none of the three needed a character changed.The same property keeps the file out of
packages/components/tsconfig.test.json, and that is deliberate rather than incidental: turbo'stype-checkwaits on^build, so a type program that read this package's owndistwould demand an artifacttype-checkis not allowed to wait for — the coupling #4801 removed. Measured with the package's own type program, with a control:Gates — union re-run at commit
7a6eef7b9vitest run scripts/Test Files 94 passed (94)/Tests 2645 passed (2645)pnpm --filter @object-ui/components run type-checkpnpm check:control-bytespnpm type-check:scriptspnpm check:doc-fencespnpm check:doc-typespnpm docs:check-linkscheck-changeset-presencecheck-changeset-overwrite/-no-major/-fixedeslinton the changed filesEvery exit code was captured before any pipe. The lint run is narrowed and the narrowing is measured: the population is the diff, the count comes from
--format json(3 files), andeslint.config.jscontains 0projectService/parserOptions/project:occurrences against a live control of 10ruleshits in the same file — not type-aware, so no untouched file's verdict can move under this diff.Changeset: an empty frontmatter, which the presence gate names as a pass rather than a workaround. The one file it flags is under
packages/components/src/but ships nowhere — the package publishesfiles: ["dist", …]and its buildtsconfig.jsonexcludessrc/__tests__outright.Corrections and deviations, stated rather than buried
The first run of all three verdicts was invalid and was thrown away. The pin file's docstring contained a glob pairing a star with a slash, which ends a block comment early; the file failed to parse, so verdict (i) was red and (ii)/(iii) reported
Tests no tests— a transform error, not an assertion.vitest list --filesOnlyhad passed because it never transforms. Fixed, and all three verdicts above are from the re-run. This repo already records the same trap intsconfig.scripts.json's header and incheck-changeset-presence.mjs.Three files beyond the dispatch's declared surface, each forced by a gate or by turbo's shape:
packages/components/package.json— one script. Turbo tasks are per-package, sodependsOn: ["build"]for the package under test cannot be expressed without a script in that package. It carries--root ../..because the invocation guard refuses any run whose Vitest root is not the repo root, and--config vitest.config.mtsbecause Vite resolvesconfigFilerelative toroot, not cwd (measured:../../vitest.config.mtsresolved to/home/vitest.config.mts).content/docs/guide/ci-cd-pipeline.md— one cell.scripts/__tests__/ci-cd-pipeline-doc.test.tsfails when the workflow runs a first-party command the job table does not name. It also fails if the cell names a command the job does not run, which is why the turbo invocation is described in prose there rather than as a code span.scripts/__tests__/turbo-task-guard-coverage.test.ts— one docstring line. That test enforces the partition "cacheable ⇒ has a derived inputs guard; otherwisecache: false", and its docstring enumerates the partition.test:distiscache: false— a lane whose subject is a build artifact turbo does not hash must never replay a verdict — so it belongs on the uncached side, and the enumeration now says so.⛔ No change to the root alias map. ⛔ No timeout raised. ⛔ No heavy-test allowlist entry. ⛔ No
skip-changesetlabel — in this repo that label is inert and the empty-frontmatter changeset is the declaration that counts.Not in this PR
Nothing migrates into the new lane. Its scarcity is the guard against it becoming a second default test surface, and the ruling asks for exactly one resident.
Fix round —
Type Checkwas red on the first pushWhat was red.
@object-ui/console#type-check, run 33588494358 / job 100117481182. Three errors, all one cause:Cause, one line. Inside the conditional spread that declares the
distproject, the literalextends: truewidens toboolean, whileTestProjectConfiguration.extendsisstring | true | undefined— so the element matched nodefineConfigoverload and the wholeprojectsarray degraded tonever[], which then took down the console entry on line 325 too.Fix.
extends: true as const, plus a comment recording why the annotation is load-bearing (it reads like removable noise). Nothing else changed; the three pre-existing projects never widened, because theirextends: truesits in the plain array rather than in a conditional one.Why CI saw it and this PR's own gates did not. The root
vitest.config.mtsis compiled by no type program of its own —apps/console/vitest.config.tsmerges it, so the console'stsc --noEmitis the program that reads it. That job was outside the gate set run before the first push. It is in the set now.Reproduced before fixing, not after. On an unbuilt worktree the real errors are unreachable behind 378
TS2882s, so the dependency closure was built through turbo first (turbo run type-check --filter=@object-ui/console), which is the path CI takes:error TSlinesRe-run at the fix commit
857c8afc5(exit codes captured by redirect-then-$?):pnpm --filter @object-ui/console run type-checkerror TSlines (was exit 1 / 3)pnpm --filter @object-ui/components run type-checkpnpm type-check:scriptspnpm type-check:vitest-setuppnpm type-check(turbo, every package)Tasks: 81 successful, 81 totalpnpm test:distTest Files 1 passed (1),Tasks: 9 successful, 9 totalvitest list --filesOnly --project distunit/dom/dom-heavystill 0 (files 810 / 1429 / 34)vitest run --project distwithout the env varNo rebase and no force-push: the fix is a commit on top of the branch.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BGMDbrVa8JjZcCQ7DWYH1b