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([]); + }); +});