chore: run each client's test suite once in local:gate, not twice (#2341)
`local:gate` ran every client's unit suite bare inside `validate` and again instrumented inside `coverage` — ~80s of a ~370s run re-executing exactly the files the coverage pass runs a few minutes later. CI pays nothing for that duplication because its two jobs run in parallel; the gate runs serially, so it paid all of it. The gate's first stage is now `local:validate`: the same `validate:guards` and `validate:core`, then each client's new `check` script (`format:check` + `lint` + `typecheck` + `build`) instead of its `validate`, which is now literally `check && test`. `npm run validate` and both CI jobs are unchanged; `local:validate` sits in the `local:` namespace so the workflow guard keeps it out of CI by construction. The "strict superset" claim in AGENTS.md is reworded to what is now true — every CI *check*, not every CI *invocation* — with the reasoning: `@vitest/coverage-v8` reads V8's precise-coverage profiler and rewrites no source, so the instrumented pass runs byte-identical code and is only slower, the stricter direction for the failure class this repo sees. Measured on the same worktree: 370s -> 262s wall (EXIT=0 both), the four removed legs summing to 78.5s in the baseline run's own profile. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall
parent
856729fb4b
commit
fe8b039030
15 changed files with 163 additions and 31 deletions
|
|
@ -30,10 +30,17 @@ review, which is the argument for not making a fourth. `npm run local:gate`
|
|||
prints each stage as it starts, so the running command is the other reliable
|
||||
answer.
|
||||
|
||||
It is a **strict superset** of GitHub CI (which additionally runs `npm install`,
|
||||
and runs `coverage` as a parallel job). So the direction that matters holds:
|
||||
**passing `local:gate` locally means CI's gates will pass.** The reverse does
|
||||
not.
|
||||
It runs **every check** GitHub CI runs (which additionally runs `npm install`,
|
||||
and runs `coverage` as a parallel job), plus two local-only steps. So the
|
||||
direction that matters holds: **passing `local:gate` locally means CI's gates
|
||||
will pass.** The reverse does not.
|
||||
|
||||
One difference in *invocations*, not checks: CI runs each client's unit suite
|
||||
twice — bare inside `validate`, instrumented inside `coverage` — on two
|
||||
parallel runners, while the gate runs it **once**, instrumented (#2341). The
|
||||
gate's first stage is `local:validate`, which is `validate` minus each client's
|
||||
`test` leg; `npm run validate` itself is unchanged. The reasoning is in
|
||||
[`AGENTS.md`](../../../AGENTS.md#mandatory-pre-push-gate).
|
||||
|
||||
⚠️ **There is no `npm run ci`.** The gate was renamed to `local:gate` (#2146)
|
||||
precisely because `npm ci` is a built-in that clean-installs from the lockfile
|
||||
|
|
|
|||
11
.github/workflows/main.yml
vendored
11
.github/workflows/main.yml
vendored
|
|
@ -49,7 +49,9 @@ jobs:
|
|||
# (#2159), which consumes nothing this job produces — every client's
|
||||
# `test:coverage` builds whatever it needs itself. Unit tests run in
|
||||
# both jobs (fast here, instrumented there); now that the two run in
|
||||
# parallel that duplication costs no wall clock at all.
|
||||
# parallel that duplication costs no wall clock at all. (The local gate,
|
||||
# which runs serially, calls `local:validate` instead and runs them once
|
||||
# — #2341. Keep `validate` here: this job IS the fast inner loop.)
|
||||
# A future optimization could split `coverage` into per-client parallel
|
||||
# jobs, but that's a larger restructure and deliberately out of scope.
|
||||
run: npm run validate
|
||||
|
|
@ -109,9 +111,10 @@ jobs:
|
|||
|
||||
- name: Run cross-client smokes
|
||||
# NOTE: this workflow runs GitHub CI's tier only. The LOCAL pre-push
|
||||
# gate is `npm run local:gate`, and it is a strict superset — do not add
|
||||
# it, a `local:*` script, `smoke:web:firefox`/`smoke:web:webkit`,
|
||||
# `smoke:web:engine`, or a non-Chromium `SMOKE_BROWSER` to any workflow.
|
||||
# gate is `npm run local:gate`, and it runs every check here plus the
|
||||
# local-only ones — do not add it, a `local:*` script,
|
||||
# `smoke:web:firefox`/`smoke:web:webkit`, `smoke:web:engine`, or a
|
||||
# non-Chromium `SMOKE_BROWSER` to any workflow.
|
||||
# `npm run test:scripts` fails if you do (scripts/lib/workflow-gate.mjs,
|
||||
# #2146). `npm run smoke` and `smoke:web:chromium` belong here and are
|
||||
# not affected. The canonical CI-vs-local table is in the root README.
|
||||
|
|
|
|||
|
|
@ -473,7 +473,8 @@ free to run a full `local:gate`). Raising a budget nobody chose hides no race.
|
|||
## Mandatory pre-push gate
|
||||
|
||||
- **ALWAYS run `npm run format` before committing.** The **root** `format` auto-fixes `core/`, the root `scripts/` tooling, the root shared surface, and every client's scope in one shot. `validate` runs the non-fixing `format:check` and will fail in CI on any unformatted file, so run the auto-fixer first rather than letting `format:check` catch it.
|
||||
- **`npm run local:gate` is the mandatory pre-push command.** It is a **strict superset** of `.github/workflows/main.yml`, so passing it locally means CI's gates will pass. Expect several minutes.
|
||||
- **`npm run local:gate` is the mandatory pre-push command.** It runs **every check** `.github/workflows/main.yml` runs, plus two local-only steps, so passing it locally means CI's gates will pass. Expect several minutes.
|
||||
- **The gate runs each client's test suite once, instrumented; CI runs it twice (#2341).** CI's `build` job runs the bare `test` inside `validate` and its parallel `coverage` job runs `test:coverage`, which costs CI no wall clock. Serially in one process the bare pass was ~80s of a ~370s gate, re-running exactly the files the coverage pass runs a few minutes later — so the gate calls **`local:validate`**, which is `validate` minus each client's `test` leg (every client's `validate` is `check && test`, and `local:validate` runs the `check` half). "Every check" is therefore a claim about checks, not invocations, and it holds because `@vitest/coverage-v8` collects coverage from V8's own profiler (`Profiler.takePreciseCoverage`) and rewrites no source: a test file sees identical code either way, and the instrumented run is only *slower*, which makes it the stricter of the two for the failure class this repo actually sees (a correct test cut off under load). A test that passed only *because* it ran slower would be a race — #1596's class, a defect wherever it surfaces — and CI's bare pass still runs it. **`npm run validate` is unchanged**, because CI runs it directly and it is the inner-loop check; `verify:*` guards that ask "is this reachable from `validate`" are unaffected. `local:validate` lives in the `local:` namespace so the workflow guard keeps it out of CI by construction.
|
||||
- **`npm run validate` is the fast inner-loop check and is NOT an acceptable substitute.** It runs `test`, not `test:coverage`, so it does **zero** coverage gating, no smokes, and no Storybook tests. Skipping the gate is how a push passes every fast local check and still fails CI.
|
||||
- There is deliberately **no `npm run ci`** — that name collided with the `npm ci` built-in, which clean-installs from the lockfile and does not run this script.
|
||||
- What each stage covers, and why two of them are local-only, is [`docs/quality-gate.md`](./docs/quality-gate.md); how to diagnose a failing stage is the `pre-push-gate` skill.
|
||||
|
|
|
|||
|
|
@ -88,7 +88,7 @@ Each client self-validates from its own folder; the root scripts chain them. The
|
|||
```bash
|
||||
npm run validate # fast inner loop: format:check + lint + typecheck + build + unit tests
|
||||
npm run coverage # the per-file ≥90% gate (lines/statements/functions/branches)
|
||||
npm run local:gate # MANDATORY before pushing — a strict superset of GitHub CI
|
||||
npm run local:gate # MANDATORY before pushing — every GitHub CI check, plus two local-only ones
|
||||
```
|
||||
|
||||
`npm run local:gate` chains every check below, plus the smokes and the Storybook tests. [Testing and the quality gate](./docs/quality-gate.md) owns the stage list and says what each one covers and why two are local-only; [`AGENTS.md`](./AGENTS.md) holds the testing rules themselves.
|
||||
|
|
|
|||
|
|
@ -472,16 +472,19 @@ While the Web Client provides a rich visual interface, the CLI is designed for:
|
|||
Like the other clients, the CLI self-validates from its own folder:
|
||||
|
||||
```bash
|
||||
npm run validate # format:check && lint && typecheck && test (fast; no coverage gate)
|
||||
npm run check # format:check && lint && typecheck (no tests)
|
||||
npm run validate # check && test (fast; no coverage gate)
|
||||
npm test # build test-servers + binary, then run all tests
|
||||
npm run test:coverage # build + tests under the per-file ≥90 coverage gate
|
||||
```
|
||||
|
||||
The CLI's `test` / `test:coverage` **build the binary first** (out-of-process
|
||||
`e2e.test.ts` spawns it). `validate` is `format:check && lint && typecheck && test`
|
||||
with no separate `build` step (`pretest` builds). Repo-root `validate:cli`
|
||||
delegates here; the coverage gate is `npm run coverage` / `coverage:cli` (also in
|
||||
`npm run local:gate`), matching AGENTS.md.
|
||||
`e2e.test.ts` spawns it). `check` is `format:check && lint && typecheck` and
|
||||
`validate` is `check && test`, with no separate `build` step (`pretest` builds).
|
||||
Repo-root `validate:cli` delegates to `validate`, and the root `local:validate`
|
||||
— the first stage of `npm run local:gate` — runs `check` instead, so the gate
|
||||
runs the suite once, under `coverage` / `coverage:cli` (#2341), matching
|
||||
AGENTS.md.
|
||||
|
||||
Tests run the CLI **in-process** (importing `runCli()`) so `src/` is measured
|
||||
under coverage, with a thin out-of-process spawn layer for the real binary. See
|
||||
|
|
|
|||
|
|
@ -17,7 +17,8 @@
|
|||
"scripts": {
|
||||
"build": "tsup",
|
||||
"typecheck": "tsc --noEmit -p tsconfig.json && tsc --noEmit -p tsconfig.test.json",
|
||||
"validate": "npm run format:check && npm run lint && npm run typecheck && npm run test",
|
||||
"check": "npm run format:check && npm run lint && npm run typecheck",
|
||||
"validate": "npm run check && npm run test",
|
||||
"test": "vitest run",
|
||||
"test:watch": "vitest",
|
||||
"test:coverage": "npm run test-servers:build && npm run build && vitest run --coverage",
|
||||
|
|
|
|||
|
|
@ -110,7 +110,8 @@ stale fixture reports a product failure rather than a staleness one.
|
|||
Like the web client, the launcher self-validates from its own folder:
|
||||
|
||||
```bash
|
||||
npm run validate # format:check && lint && build && test:coverage
|
||||
npm run check # format:check && lint && typecheck && build (no tests)
|
||||
npm run validate # check && test
|
||||
```
|
||||
|
||||
This has **no** dependency on the other clients being built — it only checks the
|
||||
|
|
@ -118,7 +119,9 @@ launcher's own source. `eslint.config.js` is a Node-only flat config (the web
|
|||
client's React/Storybook plugins stripped out), and the per-file coverage gate
|
||||
covers `parse-launcher-argv.ts` (the pure arg-parsing logic); `src/index.ts` is
|
||||
excluded as binary bootstrap and is instead exercised by the smokes above. The
|
||||
repo-root `validate:launcher` simply delegates here (`cd clients/launcher && npm run validate`).
|
||||
repo-root `validate:launcher` simply delegates here (`cd clients/launcher && npm run validate`),
|
||||
and the root `local:validate` — the first stage of `npm run local:gate` — runs
|
||||
`check` instead, so the gate runs the suite once, under `coverage:launcher` (#2341).
|
||||
|
||||
## Publishing
|
||||
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@
|
|||
"format:check": "prettier --check src __tests__ scripts \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"",
|
||||
"test": "vitest run",
|
||||
"test:coverage": "vitest run --coverage",
|
||||
"validate": "npm run format:check && npm run lint && npm run typecheck && npm run build && npm run test"
|
||||
"check": "npm run format:check && npm run lint && npm run typecheck && npm run build",
|
||||
"validate": "npm run check && npm run test"
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -93,12 +93,15 @@ The TUI provides terminal-native tabs and panes for interacting with your MCP se
|
|||
Like the other clients, the TUI self-validates from its own folder:
|
||||
|
||||
```bash
|
||||
npm run validate # format:check && lint && build && test:coverage
|
||||
npm run check # format:check && lint && typecheck && build (no tests)
|
||||
npm run validate # check && test
|
||||
npm test # run all tests
|
||||
npm run test:coverage # run tests under the per-file coverage gate
|
||||
```
|
||||
|
||||
The repo-root `validate:tui` just delegates here. `eslint.config.js` registers
|
||||
The repo-root `validate:tui` just delegates here; the root `local:validate`
|
||||
(the first stage of `npm run local:gate`) runs `check` instead, so the gate
|
||||
runs the suite once, under `coverage:tui` (#2341). `eslint.config.js` registers
|
||||
`react-hooks` for the classic rules only (rules-of-hooks + exhaustive-deps); the
|
||||
stricter react-hooks@7 rules are not enforced on the interim component surface
|
||||
(#1501).
|
||||
|
|
|
|||
|
|
@ -19,7 +19,8 @@
|
|||
"dev": "vite-node --config vitest.config.ts dev.ts",
|
||||
"build": "tsup",
|
||||
"typecheck": "tsc --noEmit -p tsconfig.json && tsc --noEmit -p tsconfig.test.json",
|
||||
"validate": "npm run format:check && npm run lint && npm run typecheck && npm run build && npm run test",
|
||||
"check": "npm run format:check && npm run lint && npm run typecheck && npm run build",
|
||||
"validate": "npm run check && npm run test",
|
||||
"test": "vitest run",
|
||||
"test:coverage": "vitest run --coverage",
|
||||
"lint": "eslint . --max-warnings 0",
|
||||
|
|
|
|||
|
|
@ -15,7 +15,8 @@
|
|||
"build:client": "vite build",
|
||||
"build:runner": "tsup --config tsup.runner.config.ts",
|
||||
"build:storybook": "storybook build",
|
||||
"validate": "npm run format:check && npm run lint && npm run build && npm run test",
|
||||
"check": "npm run format:check && npm run lint && npm run build",
|
||||
"validate": "npm run check && npm run test",
|
||||
"pretest": "npm run test-servers:build",
|
||||
"test": "vitest run --project=unit",
|
||||
"test:watch": "vitest --project=unit",
|
||||
|
|
|
|||
|
|
@ -12,15 +12,17 @@ Each client self-validates from its own folder; the root scripts chain them. The
|
|||
| Tier | How it runs | What it covers |
|
||||
| --- | --- | --- |
|
||||
| **GitHub CI** (`.github/workflows/main.yml`) | Automatically, on every push | `npm install`, then `validate`, `verify:skills:cli`, `verify:build-gate`, `verify:bundle-externals`, `smoke` (which includes `smoke:web:chromium`), `test:storybook` — plus `coverage` in a parallel job ([#2159](https://github.com/modelcontextprotocol/inspector/issues/2159)) |
|
||||
| **The local gate** (`npm run local:gate`) | By hand, before you push | Every check above (the install is yours to run), **plus** the Firefox engine pass (`smoke:web:firefox`), and `smoke:tui` for real rather than self-skipped |
|
||||
| **The local gate** (`npm run local:gate`) | By hand, before you push | Every check above (the install is yours to run; `local:validate` stands in for `validate`, see below), **plus** the Firefox engine pass (`smoke:web:firefox`), and `smoke:tui` for real rather than self-skipped |
|
||||
|
||||
The local gate is a **strict superset**, not a mirror. Two of its steps have no GitHub CI counterpart, each for its own reason:
|
||||
The local gate runs **every check** CI runs, and is not a mirror. Two of its steps have no GitHub CI counterpart, each for its own reason:
|
||||
|
||||
| Local-only step | Why it is local-only |
|
||||
| --- | --- |
|
||||
| `smoke:web:firefox` | Trialled as a CI job and removed ([#2086](https://github.com/modelcontextprotocol/inspector/issues/2086)): across a dozen runs it never once disagreed with Chromium, and `playwright install --with-deps` carries a real flake surface. Kept in front of a human about to push instead. `smoke:web:webkit` is worse still — it fails two of the three smokes for reasons nobody has identified — so it is in neither tier. See [Supported browsers](#supported-browsers). |
|
||||
| `smoke:tui` | The Ink TUI needs a real TTY. It _is_ invoked in CI via `npm run smoke` and self-skips there on `process.env.CI`, so it needs no guarding — it handles itself. |
|
||||
|
||||
And one CI *invocation* the gate deliberately does not repeat: CI runs each client's unit suite **twice** — bare inside `validate` in the `build` job, instrumented inside `coverage` in the parallel job — at no wall-clock cost, because the two jobs run on separate runners. Serially in one process that second pass was ~80s of a ~370s gate, re-running exactly the test files the coverage pass runs a few minutes later ([#2341](https://github.com/modelcontextprotocol/inspector/issues/2341)). So the gate's first stage is **`local:validate`**: the same guards and `validate:core`, then each client's `check` (`format:check` + `lint` + `typecheck` + `build`) instead of its `validate` (`check` + `test`). The suites still run once, under `coverage`. That subsumes the bare pass because `@vitest/coverage-v8` reads V8's own precise-coverage profiler and rewrites nothing — the code under test is byte-identical, and the instrumented run is only slower, which is the stricter direction for the failures this repo sees. `npm run validate` and both CI jobs are unchanged.
|
||||
|
||||
So the direction that matters holds: **passing `npm run local:gate` means CI's gates will pass.** The reverse does not.
|
||||
|
||||
**The gate is deliberately not named `ci`** ([#2146](https://github.com/modelcontextprotocol/inspector/issues/2146)). That name collided with `npm ci` — a built-in that clean-installs from the lockfile, does **not** run this script, and is one keystroke away — and it invited the cross-engine passes into GitHub CI by looking like the thing a workflow ought to call. There is **no back-compat alias**: `npm run ci` now fails with npm's missing-script error, which prints the available scripts.
|
||||
|
|
@ -41,7 +43,8 @@ That is the readable half, and prose rots. The enforced half is `scripts/lib/wor
|
|||
| `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. |
|
||||
| `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from **what actually enters each program** (#1965) — every client tsconfig project listed with `tsc --listFilesOnly` via the shared `scripts/lib/tsc-program.mjs`, each resolved `node_modules` file mapped to its owning install, keeping the packages that reach one program from two installs (a package whose declarations arrive only through another package's `.d.ts`, as `@modelcontextprotocol/sdk`'s do, is invisible to a scan of first-party imports). Prices each copy from the lockfile entry for the exact install path the program resolved, compares only the installs that met in one program, and **fails deny-by-default** on any disagreement not in the annotated `TOLERATED_SKEW` allowlist — empty today — with an allowlisted package tolerated only *within a major version*. A **second tier** (#2226) runs alongside it, asking the weaker but broader question the `AGENTS.md` rule actually states: does a package this repo *declares* anywhere resolve to two versions across our installs at all? Its candidate set is every name in any install's `dependencies`/`devDependencies`/`optionalDependencies` — unioned across the root and all four clients, so a copy declared by only one of them still counts — that **more than one install holds a top-level copy of** (17 packages today). Nested copies are excluded: one exists because some dependency asked for a different version, so it is that dependency's range to govern, not ours. Neither tier subsumes the other — the program tier sees a copy no manifest names (`@modelcontextprotocol/sdk`, arriving through another package's `.d.ts`), while the declared tier sees a **transitive** copy no program loads (cli's `@types/node`, hoisted via `@types/express` — the case that motivated it), two **clients** disagreeing with no root copy involved (`@types/react`, web against tui), and the peer shadows `eslint`/`typescript`/`vitest` that never enter a program. Same deny-by-default and same within-a-major rule, against its own `TOLERATED_DECLARED_SKEW` — also empty. Two limits: it reads **lockfiles**, so an uncommitted hand-installed copy is invisible, and it compares only declared names, so a purely transitive package no manifest names stays the first tier's business. Runs in `validate`.
|
||||
| `npm run verify:test-timeouts` | Guards the wall-clock budgets the test gates run under (#2323). The class it encodes against is a budget **nobody chose**: three of the six Vitest projects ran on Vitest's own `testTimeout: 5000` and five on its `hookTimeout`/`teardownTimeout: 10000`, sized for an idle machine rather than the one this team works on — three or four concurrent agent sessions in separate worktrees, each free to run a full `local:gate`, on eight logical cores. A correct, deterministic test cut off by such a budget fails a gate its diff did not break, which is the same "channel nobody trusts" failure [Lint has no warning tier](../AGENTS.md#lint-has-no-warning-tier) describes from the other direction; #2292, #1942 and #1742 were each an instance, found one site at a time. Deliberately **not** cited: #2278 (a missing condition wait around a geometry read) and #2250 (a real race in a test's own timing) were fixed by making the test wait for the right thing, and presenting a race fix as evidence for a larger ceiling would argue against #1596. It asks **Vitest itself** to resolve each of the four configs and reads the number a test actually gets, rather than checking that a key is absent from a config block — which would pass just as happily on a config that had stopped being loaded. A **seventh project** with no row, or a config file it does not discover (it knows all twelve filenames Vitest accepts, not just the two this repo uses), is an error rather than a silent skip. It also checks that every project loads `vitest.setup.shared.mts`. ⚠️ **Everything it reads comes from a resolved project, never from source text** — the two rules no config can report are asserted at runtime instead: `retry` by `vitest.setup.shared.mts` (which reads the value Vitest resolved, so a per-test option, a `describe` option, a project setting and a `--retry` flag are one check) and Testing Library's `asyncUtilTimeout` by `clients/web/src/test/asyncUtilTimeout.test.ts` (which reads what the project's own `waitFor`s use). Both began as source scanning in #2334 and the review found a new valid spelling missed in five consecutive rounds, so the boundary is deliberate: if a rule cannot be answered by asking the tool, assert it at runtime rather than reading the source for it. Observed **red** against the pre-#2323 config before it was trusted. Runs in `validate`; its decision logic has a sibling `verify-test-timeouts.test.mjs` under `test:scripts`.
|
||||
| `npm run local:gate` | **Mandatory pre-push command.** `validate` → `verify:skills:cli` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`. A strict superset of GitHub CI — see [Two tiers](#two-tiers-github-ci-and-the-local-gate). Named `local:` rather than `ci` on purpose (#2146); there is no `npm run ci` alias. |
|
||||
| `npm run local:gate` | **Mandatory pre-push command.** `local:validate` → `verify:skills:cli` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`. Every GitHub CI check plus two local-only ones — see [Two tiers](#two-tiers-github-ci-and-the-local-gate). Named `local:` rather than `ci` on purpose (#2146); there is no `npm run ci` alias. |
|
||||
| `npm run local:validate` | The gate's first stage: `validate` with each client's `test` leg removed (#2341) — the same `validate:guards` and `validate:core`, then every client's `check` (`format:check` + `lint` + `typecheck` + `build`; each client's `validate` is `check && test`). The suites run once, under `coverage`, instead of bare here and instrumented there. In the `local:` namespace so the workflow guard keeps it out of CI, where the bare pass is free. |
|
||||
| `npm run pack:verify` | Publish smoke — see [Publishing](./publishing.md). |
|
||||
|
||||
Per-client scripts exist too (`validate:web`, `coverage:cli`, `smoke:tui`, …), plus root `validate:core` / `format:core` for the shared `core/` package, `format:scripts` for the root `scripts/` tooling, and `format:shared` / `lint:shared` for the root "shared" surface (`test-servers/src/**`, `vitest.shared.mts`, the root `eslint.config.js`). Run `npm run format` before committing — the root `format` fixes `core/`, the root `scripts/`, the shared surface, and every client; `validate` runs the non-fixing `format:check` and fails CI on any unformatted file.
|
||||
|
|
|
|||
|
|
@ -38,7 +38,8 @@
|
|||
"build:tui": "cd clients/tui && npm run build",
|
||||
"build:web": "cd clients/web && npm run build",
|
||||
"build:launcher": "cd clients/launcher && npm run build",
|
||||
"local:gate": "npm run validate && npm run verify:skills:cli && npm run coverage && npm run verify:build-gate && npm run verify:bundle-externals && npm run smoke && npm run smoke:web:firefox && npm run local:storybook",
|
||||
"local:gate": "npm run local:validate && npm run verify:skills:cli && npm run coverage && npm run verify:build-gate && npm run verify:bundle-externals && npm run smoke && npm run smoke:web:firefox && npm run local:storybook",
|
||||
"local:validate": "npm run validate:guards && npm run validate:core && cd clients/web && npm run check && cd ../cli && npm run check && cd ../tui && npm run check && cd ../launcher && npm run check",
|
||||
"local:storybook": "cd clients/web && npx playwright install chromium && npm run test:storybook",
|
||||
"verify:build-gate": "node scripts/verify-build-gate.mjs",
|
||||
"verify:bundle-externals": "node scripts/verify-bundle-externals.mjs",
|
||||
|
|
@ -46,7 +47,8 @@
|
|||
"verify:skills": "node scripts/verify-skills.mjs",
|
||||
"verify:skills:cli": "node scripts/verify-skills-cli.mjs",
|
||||
"test:scripts": "node --test \"scripts/**/*.test.mjs\"",
|
||||
"validate": "npm run verify:format-coverage && npm run verify:skills && npm run verify:typecheck-coverage && npm run verify:dep-lockstep && npm run verify:test-timeouts && npm run test:scripts && npm run validate:core && npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher",
|
||||
"validate": "npm run validate:guards && npm run validate:core && npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher",
|
||||
"validate:guards": "npm run verify:format-coverage && npm run verify:skills && npm run verify:typecheck-coverage && npm run verify:dep-lockstep && npm run verify:test-timeouts && npm run test:scripts",
|
||||
"verify:format-coverage": "node scripts/verify-format-coverage.mjs",
|
||||
"verify:dep-lockstep": "node scripts/verify-dep-lockstep.mjs",
|
||||
"verify:test-timeouts": "node scripts/verify-test-timeouts.mjs",
|
||||
|
|
|
|||
|
|
@ -7,9 +7,11 @@
|
|||
* two verify gates, `smoke` and the Storybook tests. `npm run smoke` covers
|
||||
* `smoke:launcher`, `smoke:cli`, `smoke:tui`, `smoke:web` and
|
||||
* `smoke:web:chromium` — all of which BELONG there.
|
||||
* - **`npm run local:gate`**, the pre-push gate, is a strict superset: it adds
|
||||
* the **Firefox** engine pass, and `smoke:tui` really runs there rather than
|
||||
* self-skipping. WebKit is on demand and belongs to neither tier.
|
||||
* - **`npm run local:gate`**, the pre-push gate, runs every check CI runs and
|
||||
* adds the **Firefox** engine pass, and `smoke:tui` really runs there rather
|
||||
* than self-skipping. WebKit is on demand and belongs to neither tier. It
|
||||
* runs each client's unit suite once (instrumented, under `coverage`) via
|
||||
* `local:validate`, where CI's two parallel jobs run it twice (#2341).
|
||||
*
|
||||
* The narrow thing that must never reach CI is a **non-Chromium engine pass**.
|
||||
* `smoke:web:firefox` was placed in the local gate on purpose (#2086,
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import {
|
|||
findWorkflowViolations,
|
||||
formatWorkflowViolations,
|
||||
} from "./workflow-gate.mjs";
|
||||
import { reachableScripts } from "./npm-scripts.mjs";
|
||||
|
||||
const repoRoot = join(import.meta.dirname, "..", "..");
|
||||
const workflowDir = join(repoRoot, ".github", "workflows");
|
||||
|
|
@ -609,6 +610,106 @@ describe("the gate's name", () => {
|
|||
assert.match(scripts["local:gate"], /local:storybook/);
|
||||
});
|
||||
|
||||
describe("runs each client's test suite once, not twice (#2341)", () => {
|
||||
// CI runs each client's unit suite bare (inside `validate`) and again
|
||||
// instrumented (inside `coverage`) on two PARALLEL runners, so the second
|
||||
// pass is free there. The gate runs serially, where the bare pass was ~80s
|
||||
// of a ~370s run re-executing exactly the files `coverage` runs a few
|
||||
// minutes later. So the gate's first stage is `local:validate` — `validate`
|
||||
// minus every client's `test` leg — and these pin the three properties
|
||||
// that keep it honest: the gate no longer reaches a client's bare `test`,
|
||||
// it still reaches every non-test check `validate` reaches, and `validate`
|
||||
// itself (CI's inner loop) is untouched.
|
||||
const clients = ["web", "cli", "tui", "launcher"];
|
||||
const clientScripts = Object.fromEntries(
|
||||
clients.map((c) => [
|
||||
c,
|
||||
JSON.parse(
|
||||
readFileSync(join(repoRoot, "clients", c, "package.json"), "utf8"),
|
||||
).scripts,
|
||||
]),
|
||||
);
|
||||
|
||||
it("starts from `local:validate`, and never from `validate`", () => {
|
||||
assert.match(scripts["local:gate"], /^npm run local:validate && /);
|
||||
assert.doesNotMatch(scripts["local:gate"], /\brun validate(?=$|[\s&;])/);
|
||||
// `local:validate` must not reach a client's `validate` either — that is
|
||||
// the one door through which the bare `test` leg would come back.
|
||||
const reached = reachableScripts(scripts, "local:validate");
|
||||
for (const c of clients)
|
||||
assert.ok(
|
||||
!reached.has(`validate:${c}`),
|
||||
`local:validate must not reach validate:${c}`,
|
||||
);
|
||||
assert.match(
|
||||
scripts["local:validate"],
|
||||
/\brun check(?=$|[\s&;])/,
|
||||
"local:validate must run each client's `check`",
|
||||
);
|
||||
for (const c of clients)
|
||||
assert.match(
|
||||
scripts["local:validate"],
|
||||
new RegExp(`(?:cd (?:\\.\\./|clients/)${c}|--prefix clients/${c})`),
|
||||
`local:validate must visit clients/${c}`,
|
||||
);
|
||||
});
|
||||
|
||||
it("shares the guards and `validate:core` with `validate`", () => {
|
||||
// One list of durable guards, reached by both entry points — so a guard
|
||||
// added to `validate:guards` lands in CI and in the gate together.
|
||||
const gate = reachableScripts(scripts, "local:validate");
|
||||
const inner = reachableScripts(scripts, "validate");
|
||||
for (const name of ["validate:guards", "validate:core"]) {
|
||||
assert.ok(gate.has(name), `local:validate must reach ${name}`);
|
||||
assert.ok(inner.has(name), `validate must reach ${name}`);
|
||||
}
|
||||
// Everything `validate` reaches at the root that is not a client
|
||||
// delegation is reached by the gate too.
|
||||
for (const name of inner)
|
||||
if (
|
||||
name !== "validate" &&
|
||||
!/^validate:(web|cli|tui|launcher)$/.test(name)
|
||||
)
|
||||
assert.ok(gate.has(name), `local:validate must reach ${name}`);
|
||||
});
|
||||
|
||||
it("each client's `validate` is `check` plus `test`, and `check` runs no tests", () => {
|
||||
for (const c of clients) {
|
||||
const s = clientScripts[c];
|
||||
assert.equal(
|
||||
s.validate,
|
||||
"npm run check && npm run test",
|
||||
`clients/${c} validate`,
|
||||
);
|
||||
const fromCheck = reachableScripts(s, "check");
|
||||
for (const name of fromCheck)
|
||||
assert.doesNotMatch(
|
||||
name,
|
||||
/^(pre|post)?test(:|$)/,
|
||||
`clients/${c} check must not reach ${name}`,
|
||||
);
|
||||
assert.ok(
|
||||
!/\bvitest\b/.test([...fromCheck].map((n) => s[n]).join(" ")),
|
||||
`clients/${c} check must not invoke vitest`,
|
||||
);
|
||||
// And `check` is the whole of `validate` minus the test leg: what
|
||||
// `validate` reaches is exactly itself, plus what `check` reaches, plus
|
||||
// what `test` reaches (its `pretest` build preambles included).
|
||||
assert.deepEqual(
|
||||
[...reachableScripts(s, "validate")].sort(),
|
||||
[
|
||||
...new Set([
|
||||
"validate",
|
||||
...fromCheck,
|
||||
...reachableScripts(s, "test"),
|
||||
]),
|
||||
].sort(),
|
||||
`clients/${c} validate must reach exactly check + test`,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("has no `ci` alias, not even a deprecated one", () => {
|
||||
// The rename is only worth anything if the old name is GONE (#2146): an
|
||||
// alias preserves exactly the association being scrubbed, and leaves
|
||||
|
|
|
|||
Loading…
Reference in a new issue