From 7d61508debb3b264cf6089414318c838ce5f46dd Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 03:55:41 +0000 Subject: [PATCH] ci(coverage): keep a red coverage shard's failing test names readable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ci.yml`'s `test-coverage` legs ran `pnpm test:coverage --reporter=blob --shard=N/4`. A CLI `--reporter` REPLACES the reporter set rather than adding to it, and the set it replaced was exactly `default` plus — under `GITHUB_ACTIONS=true` — `github-actions`. A red shard's log therefore ended at `blob report written to …` with no failing test name, no assertion text and no timeout message, and the shard job's only annotation was the generic `Process completed with exit code 1.` The failures survived solely inside the `coverage-blob-N` artifact, which is download-only. The shard legs now pass `--reporter=blob --reporter=default --reporter=github-actions`. Additive on purpose: the blob stays first and the threshold overrides are untouched, because the merge job is what enforces the thresholds and it has nothing to read without the blob. Pinned in both directions by `scripts/__tests__/coverage-shard-reporter-readability.test.ts`, and the pipeline page's `test-coverage` row is updated to the command it now runs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr --- ...177-coverage-shard-reporter-readability.md | 34 ++++ .github/workflows/ci.yml | 40 ++++- content/docs/guide/ci-cd-pipeline.md | 2 +- ...overage-shard-reporter-readability.test.ts | 165 ++++++++++++++++++ 4 files changed, 239 insertions(+), 2 deletions(-) create mode 100644 .changeset/9177-coverage-shard-reporter-readability.md create mode 100644 scripts/__tests__/coverage-shard-reporter-readability.test.ts diff --git a/.changeset/9177-coverage-shard-reporter-readability.md b/.changeset/9177-coverage-shard-reporter-readability.md new file mode 100644 index 0000000000..eda5d4350b --- /dev/null +++ b/.changeset/9177-coverage-shard-reporter-readability.md @@ -0,0 +1,34 @@ +--- +--- + +Restore the failing-test names to a red coverage shard's job log (objectui#9177). +CI and test only; no package is released by this change. + +`ci.yml`'s `test-coverage` legs ran `pnpm test:coverage --reporter=blob +--shard=N/4`. A CLI `--reporter` REPLACES the reporter set rather than adding to +it, and the set it replaced was exactly `default` plus — under +`GITHUB_ACTIONS=true` — `github-actions`. A red shard's log therefore ended at +`blob report written to …`, with no failing test name, no assertion text and no +timeout message, and the shard job's only annotation was the generic `Process +completed with exit code 1.` The failures survived solely inside the +`coverage-blob-N` artifact, which is download-only. objectui#8545 priced that: +two episodes in which one test file held `main`'s coverage gate unevaluated for +84 and 87 consecutive pushes, both found by a person reading a job log by hand, +days later. + +The shard legs now pass `--reporter=blob --reporter=default +--reporter=github-actions`. Measured on vitest 4.1.10 under `--shard=N/4`: the +log carries `Failed Tests`, the test names, the assertion diff and `Error: Test +timed out in …`; `github-actions` emits one `::error` annotation per failing +test, which is the only form of this an API reader gets without downloading an +artifact. + +Additive on purpose — the blob stays first and the threshold overrides are +untouched, because the merge job is what enforces the thresholds and it has +nothing to read without the blob (objectui#5403). Verified on the same four +blobs the new invocation writes: the merged report is produced and the +configured thresholds are still evaluated over it. + +`scripts/__tests__/coverage-shard-reporter-readability.test.ts` pins both +directions — dropping the readable reporters restores objectui#9177, dropping +the blob trades an unreadable failure for an unevaluated coverage floor. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4ab8c3dbb2..5dab3a560b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -961,9 +961,47 @@ jobs: # measured, not assumed). The blob carries this shard's raw coverage as # well as its test results; the coverage report proper is produced once, # by the merge. + # + # `--reporter=default --reporter=github-actions` are here because a CLI + # `--reporter` REPLACES the reporter set rather than adding to it, and the + # set it replaced is exactly this pair. Vitest 4.1.10 resolves reporters + # with: + # + # if (!resolved.reporters.length) { + # resolved.reporters.push([isAgent ? "agent" : "default", {}]); + # if (process.env.GITHUB_ACTIONS === "true") + # resolved.reporters.push(["github-actions", {}]); + # } + # + # — and this repo's vitest config sets no `reporters`, so before + # objectui#9177 the blob flag alone was the whole set. A red shard's log + # therefore ended at `blob report written to …` with no failing test name, + # no assertion text and no timeout message, and the shard job's only + # annotation was the generic `Process completed with exit code 1.` The + # failures existed solely inside the `coverage-blob-N` artifact, which is + # download-only. objectui#8545 priced that: two episodes in which ONE test + # file held main's coverage gate unevaluated for 84 and 87 consecutive + # pushes, both found by a person reading a job log by hand, days later. + # + # ⚠️ Additive on purpose — the blob stays, because the merge job below is + # what enforces the thresholds and it has nothing to read without it. + # Measured on vitest 4.1.10 with all three reporters and `--shard=N/4`: + # the log carries `Failed Tests`, the test names, the assertion diff and + # `Error: Test timed out in …`; `github-actions` emits one `::error` + # annotation per failing test, which is the only form of this an API + # reader gets without downloading an artifact; and the shard still writes + # `.vitest-reports/blob-N-4.json`, which merges and has the configured + # thresholds enforced over it exactly as before. + # + # ⛔ Do not "simplify" this back to one reporter. Dropping the blob trades + # an unreadable failure for an unevaluated coverage floor — the same + # defect wearing the other hat — and dropping the other two restores + # objectui#9177. `scripts/__tests__/coverage-shard-reporter-readability.test.ts` + # fails in both directions. - name: Run tests with coverage (shard ${{ matrix.shard }}/4) run: >- - pnpm test:coverage --reporter=blob --shard=${{ matrix.shard }}/4 + pnpm test:coverage --reporter=blob --reporter=default + --reporter=github-actions --shard=${{ matrix.shard }}/4 --coverage.thresholds.lines=0 --coverage.thresholds.functions=0 --coverage.thresholds.branches=0 --coverage.thresholds.statements=0 diff --git a/content/docs/guide/ci-cd-pipeline.md b/content/docs/guide/ci-cd-pipeline.md index bb1fa2b9d6..f4ce51c9f1 100644 --- a/content/docs/guide/ci-cd-pipeline.md +++ b/content/docs/guide/ci-cd-pipeline.md @@ -219,7 +219,7 @@ it green — which is how two of `type-check`'s gates came to be missing from th | `changeset-check` | Changeset Fixed Group Check | `scripts/check-changeset-fixed.mjs` — every workspace package must be in the changeset `fixed` group or explicitly ignored. It checks group *membership*; it does **not** check whether the PR added a changeset. | Every run | | `type-check` | Type Check | `scripts/check-type-check-coverage.mjs`, then `pnpm check:phantom-deps`, then `pnpm check:self-import`, then `pnpm check:unreferenced-sources`, then `pnpm check:doc-example-readers`, then `pnpm check:handler-key-reads`, then `pnpm check:published-tsconfig-exclude`, then `pnpm check:side-effects-array`, then `pnpm check:element-data-source-declaration`, then `pnpm check:esm-specifiers`, then `pnpm check:spec-symbols`, then `pnpm check:action-forward-parity`, then `pnpm check:designer-field-key-parity`, then `pnpm check:icon-record-names`, then `pnpm check:i18n-keys`, then `pnpm check:i18n-drift`, then `pnpm check:i18n-designer-parity`, then `pnpm type-check:scripts`, then `pnpm type-check:vitest-config`, then `pnpm type-check`, then `pnpm type-check:vitest-setup`. The coverage guard runs first because turbo silently skips packages that have no `type-check` script, so a package without one would otherwise read as passing (#2911). `pnpm check:phantom-deps` fails when a released package imports a bare specifier its own `package.json` does not declare — a *phantom dependency*, invisible locally because the workspace root's `devDependencies` sit on the upward resolution path from every package directory and on no consumer's, so `require.resolve('react', { paths: ['packages/core/src'] })` succeeds while `@object-ui/core` declares react in no field at all ([#4394](https://github.com/objectstack-ai/objectui/issues/4394)). `pnpm check:self-import` runs next because it reuses that gate's parser: it fails when a file inside a package names its OWN package, a specifier that resolves through the package's `exports` map to `dist/` while `type-check` waits on `^build` — the *dependencies'* builds, never the package's own — so on a cold cache the declarations do not exist yet and the file fails with `TS2307`. Locally it is always green, because every local workflow builds before it type-checks and leaves a `dist/` behind; PR #4789's first run was red on exactly one such line ([#4801](https://github.com/objectstack-ai/objectui/issues/4801)). `pnpm check:unreferenced-sources` runs next, reusing the same parser again: it fails when a covered package ships a source file that nothing reaches — not the package's declared entry, and not its build config. Until [#7515](https://github.com/objectstack-ai/objectui/issues/7515) no gate here could see one: `check-dist-completeness` asks whether `dist/` holds what `tsc` emits, `check-readme-exports` compares documented exports against shipped ones, and a file that is in the tarball while being reachable from nothing is outside both — so the detection mechanism was a human reading unrelated code, which is how both instances found in one week were found ([#7319](https://github.com/objectstack-ai/objectui/issues/7319), [#7397](https://github.com/objectstack-ai/objectui/issues/7397)). The hazard is not the bytes: the file #7319 removed carried the same export name as a live engine one package over and evaluated no predicate, so name-completion alone could have wired a silently wrong renderer into a published package. Reachability has TWO roots, and the second is the whole difficulty — `packages/components` reaches its two `use-sync-external-store` shims only through `vite.config.ts` `resolve.alias` entries whose importer is a bundled dependency no source file names, so a walk that skips that leg reports exactly those two live files as dead on its first run, and a gate that cries wolf gets switched off rather than fixed. Scope is DECLARED per package in `COVERED_PACKAGES` and the uncovered remainder is printed as a count derived from the workspace on every run, because the alias mechanisms differ per package and a gate that covers one package correctly beats one that covers forty with false positives. An alias expression it cannot evaluate is a FINDING rather than a skip, since skipping one would make it accuse whatever file that alias points at. `pnpm check:doc-example-readers` runs next, on the same parser again: it fails when an exported symbol's own JSDoc `@example` hand-spells a resolution that its REAL call sites obtain by calling a shared reader. A doc comment is what the next call site is copied from, so prose that outlives the ruling it encoded re-seeds every later copy — measured at two cards and three copied call sites ([#7627](https://github.com/objectstack-ai/objectui/issues/7627), [#7638](https://github.com/objectstack-ai/objectui/issues/7638)), both closed by pointing the prose at `resolveRecordSourceObjectName`. Nothing here could see either one, and `check-spec-symbol-derivation` was credited with the class twice — in #7638's card body and then in the dispatch that repeated it — while its rule 4 judges `@objectstack/spec` citations at member granularity and says nothing about prose prescribing a LOCAL spelling ([#7652](https://github.com/objectstack-ai/objectui/issues/7652)). It fires on four conditions at once — the example calls the symbol it documents, a real call site fills the same argument slot by calling an exported single-`return` reader, the example does not, and what the example writes there is that reader's own return expression or one of the rungs it resolves between — which is what keeps it off the literals and placeholders an example legitimately carries. It does NOT judge whether a prescribed spelling is correct: on the day either card was filed the prose and every copy of it agreed, and no gate reading only the tree can know a ruling. What it catches is the state right after, when the call sites move and the prose does not. `pnpm check:handler-key-reads` runs next, on the same parser again: it fails when an `on*` handler key that a REGISTERED renderer reads off the authored document is not a declared member of the zod arm for the type it is registered under. `BaseSchema` is `.passthrough()`, so an undeclared key is not refused — it stops being judged and the value is KEPT, then reaches the renderer that reads it; measured on the built dist, `{ type: 'kanban', columns: [], onCardClick: { action: 'toast' } }` went from REFUSED to ACCEPTED with the object surviving into the parsed output ([#7664](https://github.com/objectstack-ai/objectui/issues/7664)). Every gate stayed green, because the [#6124](https://github.com/objectstack-ai/objectui/issues/6124) ledger's population is two hand-written arrays of tuples and that change re-keyed the arm by SUBSTITUTION — so its length assertion held, and a count ratchet would have been green too, which is why [#7753](https://github.com/objectstack-ai/objectui/issues/7753) rejected that option on the instance itself. This gate derives BOTH populations: the arms from every `type: z.literal(…)` in `packages/types/src/zod`, and the read sites from every real `ComponentRegistry.register(…)` call — read off the AST, because one types file NAMES that call in prose eleven times and registers nothing. It follows the document one component at a time rather than every JSX child, because most children are handed a DIFFERENT document (a dashboard's widgets each get their own), and the chain it must reach is four hops long: `register('kanban', ObjectKanbanRenderer)` names a component, that component is an HOC, the document arrives at `ObjectKanban` through a render-prop parameter and at `KanbanRenderer` through an object spread. It says nothing about keys that reach a renderer only through a `{...props}` spread onto a Radix root or a DOM listener slot — there is no read site to derive from — nor about the ledger's `?: never` tombstones, which have no read site by construction; `KNOWN_UNDECLARED_READS` is an exemption list that only shrinks, each row naming the card that owns the fix, and a row whose read site the gate can no longer find fails it. It lives in `scripts/` because the read sites are spread across `@object-ui/plugin-*` and `packages/components`, which `@object-ui/types` may not import — `check:phantom-deps` rejects it and it would close a cycle. `pnpm check:published-tsconfig-exclude` follows, config reads only: it fails when a published package's build `tsconfig.json` excludes tooling by FILE NAME (`*.test.ts`) without also excluding the tooling DIRECTORIES (`**/__tests__/**` and its two siblings, derived from `TOOLING_FILE` rather than retyped). A name-only exclude stops the files that happen to be named that way and nothing else, so the first shared helper added to a `__tests__/` directory becomes a program input and an emitting program writes it into the published `dist` — three times so far, each found by a human and never by a gate ([#4006](https://github.com/objectstack-ai/objectui/issues/4006), [#4836](https://github.com/objectstack-ai/objectui/issues/4836), [#6943](https://github.com/objectstack-ai/objectui/issues/6943), the third in the same package as the first). [#7212](https://github.com/objectstack-ai/objectui/issues/7212) measured the standing exposure — 29 published packages carrying the name form with ZERO offending files, green because nobody had added such a helper yet — and the gate landed together with their conversion so `main` was green on merge. It reads `exclude` arrays and nothing else: no build, no artifact, no emit model, which is the narrower scope that keeps it clear of the modelling [#4846](https://github.com/objectstack-ai/objectui/issues/4846) declined for the artifact-level gate. Six published packages are named carve-outs, each re-proving its own reason on every run: `cli`, `create-plugin` and `data-objectstack` emit from a `tsup` entry graph, `plugin-charts` keeps its tooling exclude in the `dts()` options, and `console` and `runner` are Vite applications with `noEmit: true` and no `dts()` plugin. `pnpm check:side-effects-array` runs next, sources only and no build: it fails when a package's `sideEffects` ARRAY and its module bodies disagree in either direction — a module that registers something at load time and is not named (a bundler drops it, and the registration is gone from a *consumer's* app with no error, no warning and exit 0), or a name whose module no longer registers anything. `@object-ui/app-shell` declares such an array because both simpler answers are measurably wrong for it: omitting the field makes the whole package unshakeable, and `"sideEffects": false` silently drops three live SDUI widget registrations to zero chunks ([#6535](https://github.com/objectstack-ai/objectui/issues/6535), [#6683](https://github.com/objectstack-ai/objectui/issues/6683)). The enumeration is re-derived from the module bodies on every run rather than listed, so there is no second copy to rot. The artifact half of the same contract — do those registrations survive a real bundler — cannot run in this job at all: it needs a built console, so it lives in the SDUI registration pin step of `performance-budget.yml`. `pnpm check:element-data-source-declaration` runs next, sources only and no build: it fails when a source that consumes `ElementDataSourceGate` does not also pass through `elementDataSourceBlock()`, the seam that declares the `dataSource` key the gate reads. A block that wraps the gate off-seam publishes an authoring surface missing the one key its own runtime honours, and the html tier reports that key with the same `unknown-prop` warning it gives the spellings that do nothing ([#6678](https://github.com/objectstack-ai/objectui/issues/6678)). `pnpm check:esm-specifiers` follows it for the same reason — sources only, no build: it fails when a published package whose build preserves import specifiers (a bare emitting `tsc`, which never rewrites them) writes a relative specifier with no file extension. Node's ESM resolver does not extension-search relative specifiers, so such a specifier makes the published entry unloadable outside a bundler; `@object-ui/react`'s entry died with `ERR_MODULE_NOT_FOUND` while every bundler-based consumer, the whole test suite and CI stayed green ([#4538](https://github.com/objectstack-ai/objectui/issues/4538)). The half that actually *imports* each built entry needs a full build and runs in `node-esm-load-gate.yml`. `pnpm check:action-forward-parity` fails when an action renderer's forward whitelist drops a key the action runtime reads — the class that shipped six times one key at a time, each time green, because the key parses and publishes while the payload is dropped one hop before the runner ([#4050](https://github.com/objectstack-ai/objectui/issues/4050)). `pnpm check:designer-field-key-parity` fails when one of the field designers' statically declared payload shapes (`FieldMetadataPayload`, `ServerFieldSchema`, `DesignerFieldDefinition`) declares a key the installed `@objectstack/spec` `FieldSchema` refuses by NAME. Such a key makes `PUT /api/v1/meta/object/:name` return a hard 422 `INVALID_METADATA` that blocks *every subsequent save* of that object, and the author cannot tell from the designer UI which key did it — the class had been filed three times, each closed with a per-key tombstone written after the instance was found in production, with nothing detecting the next one ([#4644](https://github.com/objectstack-ai/objectui/issues/4644) `indexed`, [#4687](https://github.com/objectstack-ai/objectui/issues/4687) `distance_metric`, [#4676](https://github.com/objectstack-ai/objectui/issues/4676) `placeholder`, gated by [#5761](https://github.com/objectstack-ai/objectui/issues/5761)). It reads the accept set off the schema itself rather than from a list, and it covers a deliberately documented *subset* of the write path: a key that reaches the payload only through a `patchDef` spread or an index signature is outside its reach, and the boundary is stated in the script's own docblock. Its draft-I/O half — the `readFields`/`writeFields` round-trip, which has no declared shape to read — runs in the test suite as `object-fields-io.spec-keys.test.ts`. Same placement rationale as the gates around it: it parses the sources with `typescript` and imports the installed spec, so it needs the install and nothing built. `pnpm check:icon-record-names` fails when an authored icon NAME that reaches a resolver reading lucide's runtime `icons` record is not a live key of that record. lucide retires a spelling by dropping it from that record while keeping it as a deprecated named export, so the retired name still imports, still type-checks and still renders wherever it is used as a *component* — `Edit === SquarePen` is true — and resolves to nothing wherever it is used as a *string*: nothing goes red in either direction, which is why the class was repaired twice in two packages before anyone gated it ([#5586](https://github.com/objectstack-ai/objectui/issues/5586), [#5622](https://github.com/objectstack-ai/objectui/issues/5622), [#5633](https://github.com/objectstack-ai/objectui/issues/5633)). It carries no list of retired spellings — the record itself is the judgement — and it re-discovers the resolver population from source on every run, which is how its first pass found four record-reading resolvers nobody had catalogued. It sits here because it parses the sources with `typescript` and reads the installed lucide: the install, and nothing built. The three locale gates sit in the middle because all of them parse the sources with `typescript`: they need the install and nothing built. `pnpm check:i18n-keys` fails when a `t()` call site asks for a key the `en` pack does not define ([#3530](https://github.com/objectstack-ai/objectui/issues/3530)); `pnpm check:i18n-drift` fails when a change to an `en` string is not accompanied by the nine translation packs ([#3650](https://github.com/objectstack-ai/objectui/issues/3650)), and it is why this job's checkout sets `fetch-depth: 0` — it diffs against the merge base, which a depth-1 clone cannot resolve. `pnpm check:i18n-designer-parity` fails when the metadata-admin designer's own module-local string tables come apart — an `en` row with no `zh` row, or a shared row whose two values carry different `{placeholders}` ([#8834](https://github.com/objectstack-ai/objectui/issues/8834)). The two gates before it are blind to that file by construction — the first classifies the module `module-local table` by declaration and skips it, the second read only the ten locale packs and this table is not one of them — so the same card also gave `pnpm check:i18n-drift` that table as a SECOND population, which is the half that catches a changed `en` value whose `zh` row did not follow. `pnpm type-check:scripts` (`tsconfig.scripts.json`) covers `scripts/**/*.ts`, which `pnpm type-check` cannot reach at all — `scripts/` has no package.json, so turbo never walks it, and the coverage guard decides coverage per *package*. Until [#3494](https://github.com/objectstack-ai/objectui/issues/3494) that left the pin tests in `scripts/__tests__/` — including the one pinning this very page — compiled by nothing. `pnpm type-check:vitest-config` runs `apps/console/tsconfig.node.json` directly — the program that already lists `../../vitest.config.mts` and `apps/console/vitest.config.ts` ([#3476](https://github.com/objectstack-ai/objectui/issues/3476)) — because until [#7328](https://github.com/objectstack-ai/objectui/issues/7328) the only thing that ran it was the console's own `type-check` script, reached through the task runner, whose `type-check` task waits on `^build`. (Named in prose rather than as a code span on purpose: the pin below reads this cell as this job's gate list, so spelling that invocation out would credit the job with a command it does not run.) The cheapest compiler that reads the root Vitest config was therefore reachable only through the most expensive job here, and PR #7291 paid for it: a conditionally spread `dist` project whose literal `extends: true` widened to `boolean` degraded the whole `projects` array to `never[]`, every gate its author ran was green, and CI reported three errors, two of them at `../../vitest.config.mts`. It sits in the cheap half beside `pnpm type-check:scripts` for the same measured reason — nothing in its program imports an `@object-ui/*` package, so it needs the install and nothing built. `pnpm type-check:vitest-setup` (`tsconfig.vitest-setup.json`) closes the same gap for the four repo-root `vitest.setup.*` files, uncovered until [#3515](https://github.com/objectstack-ai/objectui/issues/3515); it runs *last*, after `pnpm type-check`, because `vitest.setup.dom.tsx` side-effect-imports four `@object-ui/*` packages and resolves them through the declarations that turbo's `^build` produces. | Every run; on a PR the steps short-circuit when only ignored paths changed | | `test` | Test (shard N/4) | When a pull request changed nothing outside the exclusion list, the decision step runs `scripts/markdown-test-inputs.mjs` before anything else, and the job runs in full when the answer is yes ([#8861](https://github.com/objectstack-ai/objectui/issues/8861)). The exclusions drop every markdown path, and a markdown document can be a TEST'S INPUT: [#8857](https://github.com/objectstack-ai/objectui/issues/8857) changed one package README, this job reported success in ten seconds having run nothing, and the merge-queue build then failed the same shard in 907 seconds and dequeued it. That script carries the derived class — which documents a test reads, and which test reads each — and audits itself against the tree, so the widening stays narrow: a markdown-only change nothing reads still skips. It is this job's stage only, because this is the job that runs those tests. Then `pnpm test --shard=N/4` across a 4-runner matrix with `fail-fast: false`, so every shard reports its own failures. No coverage instrumentation — v8 adds 40–100% overhead. Then, **on shard 1 only**, `pnpm test:dist` — the built-artifact lane ([#7183](https://github.com/objectstack-ai/objectui/issues/7183)). It delegates to a turbo task scoped to the one package that holds built-artifact pins; that task depends on the package's OWN build (`dependsOn: ["build"]`, not `^build`), so the bundle exists before the pins read it, and then runs the `dist` vitest project, whose pins import a package's BUILT bundle instead of its `src` — a claim the source-aliased suite above is structurally unable to make, since the root config aliases every workspace package to `src`. It is deliberately not sharded and not repeated on the other three runners: the lane is a handful of files, and running it on all four would pay for the same build four times. | Pull requests and merge-queue builds (everything but `push`); steps short-circuit on a PR that changed only ignored paths | -| `test-coverage` | Test (coverage shard N/4) | `pnpm test:coverage --reporter=blob --shard=N/4` across a 4-runner matrix with `fail-fast: false`. Each shard writes `.vitest-reports/blob-N-4.json` — raw coverage and test results in one file — and uploads it as an artifact even when the shard is red, which is what makes a failing coverage run diagnosable at all (vitest deletes `coverage/` on a red run unless `coverage.reportOnFailure` is set, [#5402](https://github.com/objectstack-ai/objectui/issues/5402)). The configured coverage thresholds are neutralised on the shard legs, because a quarter of the suite judged against a whole-suite threshold is not a defect signal; they are enforced once, on the merged report, by the job below ([#5403](https://github.com/objectstack-ai/objectui/issues/5403)). | **Push only** | +| `test-coverage` | Test (coverage shard N/4) | `pnpm test:coverage --reporter=blob --reporter=default --reporter=github-actions --shard=N/4` across a 4-runner matrix with `fail-fast: false`. Each shard writes `.vitest-reports/blob-N-4.json` — raw coverage and test results in one file — and uploads it as an artifact even when the shard is red (vitest deletes `coverage/` on a red run unless `coverage.reportOnFailure` is set, [#5402](https://github.com/objectstack-ai/objectui/issues/5402)). The two reporters after the blob are what keep a red shard *readable*: a CLI `--reporter` replaces the default reporter set rather than adding to it, so `--reporter=blob` on its own ended a failing shard's log at `blob report written to …` with no test name and no assertion text, leaving the download-only artifact as the only copy of the failure ([#9177](https://github.com/objectstack-ai/objectui/issues/9177)); `--reporter=default` restores the log and `--reporter=github-actions` restores the per-test `::error` annotation. The flags are pinned by `scripts/__tests__/coverage-shard-reporter-readability.test.ts`. The configured coverage thresholds are neutralised on the shard legs, because a quarter of the suite judged against a whole-suite threshold is not a defect signal; they are enforced once, on the merged report, by the job below ([#5403](https://github.com/objectstack-ai/objectui/issues/5403)). | **Push only** | | `coverage-report` | Test (coverage) | Downloads the four blob reports, refuses to continue unless all four arrived, merges them with `pnpm test:coverage --merge-reports` into one complete report — which is where the configured coverage thresholds are enforced, over the whole merged map, the shard legs having overridden them to zero — and publishes that report as the `coverage-report` artifact (kept 7 days, the same as the blobs it is derived from). Its last step runs on every path and states the outcome: the job is **red, with an error annotation**, whenever the gate did not run for the commit — before [#5403](https://github.com/objectstack-ai/objectui/issues/5403) the final step carried the implicit `success()` and was silently skipped by 311 of 373 coverage jobs, which is how four days of a 100%-failing coverage job went unnoticed. A breach of the thresholds is reported *separately* from a lane that never delivered, because the two call for opposite actions. ⛔ It never merges a report from fewer than four shards: a wrong coverage number is worse than a missing one. The Codecov upload this job used to carry was retired by [#5436](https://github.com/objectstack-ai/objectui/issues/5436) — `CODECOV_TOKEN` was never set, so it failed on every push; the trend dashboard and PR coverage comments are gone with it, the gate is not. | **Push only** | | `e2e` | Build & E2E | Builds the console with `vite build` (`VITE_BASE_PATH=/console/`), verifies the artifact, then `pnpm test:e2e --project=chromium`. On failure it uploads `test-results/` — the screenshots, traces and `error-context.md` Playwright writes for failing specs — as the `e2e-failure-artifacts` upload; the `github` reporter this lane runs on CI writes annotations, so there is no HTML report in it. | Every run; on a PR the steps short-circuit when only ignored paths changed | | `docs` | Build Docs | `turbo run build --filter='@object-ui/site'`. On a PR it first diffs against the base and skips the build only when nothing that build consumes changed. ⛔ The path set is **not enumerated here**, deliberately: it is derived rather than curated — from the workspace packages turbo builds on the way to `@object-ui/site`, the root-level inputs `turbo.json` declares for the `build` task, and the workspace manifests, plus the workflow file itself so a change to this gate is validated by the gate — and a path list copied into prose is a stale list the moment the closure moves, which is the class [#8629](https://github.com/objectstack-ai/objectui/issues/8629) and [#7448](https://github.com/objectstack-ai/objectui/issues/7448) each record. `scripts/__tests__/docs-build-trigger.test.ts` re-derives all three populations on every PR and executes the step's own shell against them, so the live answer is the pathspec in the step and a red test is what happens when it stops covering them. Until [#8647](https://github.com/objectstack-ai/objectui/issues/8647) the filter named the site's **output** surface only — the docs content and the site app — while its **input** surface is everything turbo builds before `next build` runs, so a pull request touching only `packages/**` skipped the build and still reported `success`, and a skipped build and a passed build are the same green to every reader downstream. The merge-queue leg always built, so what the filter cost was early detection rather than the guarantee at merge time. Then `scripts/check-doc-expression-carriage.mjs`, which is **report-only**: it censuses every `json` fence on the surface `check:doc-types` walks (`content/docs/**`, every `apps//docs/**` tree, and the root `README.md` — widened from `content/docs/**` alone by [#7878](https://github.com/objectstack-ai/objectui/issues/7878)) for a `${…}` authored on a key `SchemaRenderer` never evaluates — the class that reached `main` four times under green gates, because `check:doc-types` judges the `type` literal only and `check:doc-snippets` compiles the ts/tsx blocks only ([#7851](https://github.com/objectstack-ai/objectui/issues/7851)). It prints its findings and **exits 0 regardless**, so it can block no merge; it exits 1 only when the instrument itself is broken — a derivation that matched nothing, a missing `@objectstack/spec` artifact, or a failed built-in control — because a check that runs, goes green and looked at nothing is worse than none. Report-only is a ruling, not an oversight: three cards of the class it reports ([#7440](https://github.com/objectstack-ai/objectui/issues/7440), [#7444](https://github.com/objectstack-ai/objectui/issues/7444), [#7838](https://github.com/objectstack-ai/objectui/issues/7838)) are open and each fixes its own sites. It does **not** check docs links any more — that moved to `docs-links.yml` (#3448), because this workflow's `paths-ignore` then hid exactly the docs-only PRs a link check needs to see. #3523 has since removed that filter from the `pull_request` trigger, but the check stays in its own home: `docs-links.yml` still runs where this workflow does not (a docs-only push to `main`), and one gate with one home was the point of #3448. | Every run (build itself conditional) | diff --git a/scripts/__tests__/coverage-shard-reporter-readability.test.ts b/scripts/__tests__/coverage-shard-reporter-readability.test.ts new file mode 100644 index 0000000000..9a19eab653 --- /dev/null +++ b/scripts/__tests__/coverage-shard-reporter-readability.test.ts @@ -0,0 +1,165 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { parse as parseYaml } from 'yaml'; + +/** + * The pin on the coverage shard leg's reporter set (objectui#9177). + * + * ## What this guards, and why nothing else could + * + * `ci.yml`'s `test-coverage` job runs vitest four ways in parallel and merges + * the four blob reports in the job below it, which is where the configured + * coverage thresholds are enforced. A CLI `--reporter` **replaces** the + * reporter set rather than adding to it, and vitest's resolution is: + * + * if (!resolved.reporters.length) { + * resolved.reporters.push([isAgent ? "agent" : "default", {}]); + * if (process.env.GITHUB_ACTIONS === "true") + * resolved.reporters.push(["github-actions", {}]); + * } + * + * so `--reporter=blob` on its own displaced BOTH halves of that pair. A red + * shard's log then ended at `blob report written to …` — no failing test name, + * no assertion text, no timeout message — and the shard job's only annotation + * was the generic `Process completed with exit code 1.` The failures survived + * only inside the `coverage-blob-N` artifact, which is download-only. + * + * objectui#8545 measured what that costs: two episodes in which one test file + * held `main`'s coverage gate unevaluated for **84** and **87** consecutive + * pushes, both found the same way — a person reading a job log by hand, days + * later. + * + * Nothing else in this repository reads what that step runs. + * `ci-cd-pipeline-doc.test.ts` pins the pairing between `ci.yml` and the + * pipeline page, but it compares COMMANDS, not flags — its extractor is + * + * for (const m of text.matchAll(/\bpnpm\s+([\w:.-]+)/g)) { + * if (rootScripts.has(m[1])) found.add(`pnpm ${m[1]}`); + * } + * + * whose capture stops at the first space, so `pnpm test:coverage` is all it + * ever sees. Every flag on that line could be deleted and that file stays + * green. + * + * ## Both directions, because the failure has two shapes + * + * Dropping `--reporter=default` / `--reporter=github-actions` restores + * objectui#9177. Dropping `--reporter=blob` to "simplify" the line is the same + * defect wearing the other hat: the merge job loses the only input it has, so + * a readable failure is bought with an unevaluated coverage floor + * (objectui#5403). Neither is caught by CI going green, because the shard leg + * passing is exactly the state in which nobody looks at its log. + * + * ⛔ If you are deleting this file, you are deleting the only mechanical guard + * on that reporter set. The measurement behind it is in the ⚠️ comment above + * the step in `ci.yml`; re-run it before you change the line. + */ +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const workflowPath = path.join(repoRoot, '.github/workflows/ci.yml'); +const workflowSource = fs.readFileSync(workflowPath, 'utf8'); + +/** The reporter the merge job needs, and the two that keep the log readable. */ +const BLOB_REPORTER = '--reporter=blob'; +const READABLE_REPORTERS = ['--reporter=default', '--reporter=github-actions']; + +interface Step { + name?: string; + run?: unknown; +} + +/** + * Every `run:` body in the `test-coverage` job that invokes the coverage suite. + * + * Read through the YAML parser rather than off the raw text: the step is a + * folded block scalar (`run: >-`), so the flags are spread over four source + * lines and a regex over the file would have to re-implement the folding to see + * the command the runner actually executes. Matching on `test:coverage` rather + * than on the full string is what makes a rewrite FAIL here instead of + * vanishing — a pin that searched for today's exact line would simply stop + * finding anything and report a healthy green over a step that no longer + * contains it. + */ +function coverageRunSteps(source: string): Array<{ step: string; run: string }> { + const workflow = parseYaml(source) as { + jobs?: Record; + }; + const job = workflow.jobs?.['test-coverage']; + expect( + job, + 'ci.yml no longer defines a `test-coverage` job. If the coverage lane was restructured, this pin ' + + 'needs a new referent — do not delete it without giving the reporter set one.', + ).toBeDefined(); + const found: Array<{ step: string; run: string }> = []; + for (const step of job?.steps ?? []) { + if (typeof step.run !== 'string') continue; + if (!step.run.includes('test:coverage')) continue; + found.push({ step: step.name ?? '(unnamed)', run: step.run }); + } + return found; +} + +describe("ci.yml's coverage shards keep a red run readable", () => { + it('still runs the coverage suite at all — the pin is not vacuous', () => { + // Without this, every assertion below passes over a job that stopped running + // vitest, which is the one way a `toContain` pin lies. + const steps = coverageRunSteps(workflowSource); + expect( + steps.map((s) => s.step), + 'no step in ci.yml\'s `test-coverage` job runs `pnpm test:coverage` any more. If that is ' + + 'deliberate, this pin is obsolete and should be deleted WITH the reasoning that made it ' + + 'obsolete written down. If it is not, the push lane just lost its coverage measurement.', + ).not.toEqual([]); + }); + + it('is reading the shard leg, not some other coverage invocation', () => { + // Names the population the two assertions below judge: the sharded legs. + // `--merge-reports` in the job below is a different step in a different job + // and is deliberately out of reach here. + const sharded = coverageRunSteps(workflowSource).filter((s) => s.run.includes('--shard=')); + expect( + sharded.map((s) => s.step), + 'the `test-coverage` job no longer passes `--shard=` to the coverage suite. The reporter ' + + 'rule below is about the SHARDED legs specifically — an unsharded lane has no blob to ' + + 'keep and a different set of reporters is right for it.', + ).not.toEqual([]); + }); + + it('passes a reporter that prints failing test names alongside the blob', () => { + const offenders = coverageRunSteps(workflowSource) + .filter((s) => s.run.includes(BLOB_REPORTER)) + .flatMap((s) => + READABLE_REPORTERS.filter((r) => !s.run.includes(r)).map((r) => `${s.step}: missing ${r}`), + ); + + expect( + offenders, + 'a coverage shard passes `--reporter=blob` without the reporters that keep its log readable.\n' + + offenders.map((o) => ` - ${o}`).join('\n') + + '\n\nA CLI `--reporter` REPLACES the default set rather than adding to it, so blob alone ends a ' + + 'failing shard at `blob report written to …` with no test name and no assertion text, and leaves ' + + 'the download-only artifact as the only copy of the failure. `--reporter=default` restores the ' + + 'job log; `--reporter=github-actions` restores the per-test `::error` annotation, which is the ' + + 'only form an API reader can get without downloading anything. objectui#9177; the cost is ' + + 'objectui#8545, where one test file held the coverage gate unevaluated for 84 and 87 pushes.', + ).toEqual([]); + }); + + it('keeps the blob reporter the merge job depends on', () => { + const offenders = coverageRunSteps(workflowSource) + .filter((s) => s.run.includes('--shard=')) + .filter((s) => !s.run.includes(BLOB_REPORTER)) + .map((s) => s.step); + + expect( + offenders, + 'a sharded coverage leg no longer writes a blob report:\n' + + offenders.map((o) => ` - ${o}`).join('\n') + + '\n\nThe `coverage-report` job merges the four blobs and enforces the configured thresholds over ' + + 'the merged report — that is the whole reason the lane is sharded (objectui#5403). Restoring log ' + + 'readability by dropping the blob would trade an unreadable failure for an unevaluated coverage ' + + 'floor, which is the same defect wearing the other hat (objectui#9177).', + ).toEqual([]); + }); +});