Skip to content

Commit 6fdbbc1

Browse files
authored
ci(ui-kit): wire ui-kit's vitest suite into ui:test/CI, add test-wiring checker (#10217)
@loopover/ui-kit declares "test": "vitest run" but nothing runs it -- ui:test only ever exercised ui and ui-miner, test:ci reaches ui-kit through build/pack/lint/typecheck but never its own test script, and ci.yml has no ui-kit test step. The 12-file suite has been silently unexecuted since it was added. Add the ui-kit workspace to ui:test (after ui:kit:build) and a matching "UI tests (ui-kit)" CI step alongside the ui/ui-miner ones. Add scripts/check-test-coverage-wiring.ts, a test-script analogue of check-typecheck-coverage.ts: it walks the same npm-run reference graph to report any workspace whose declared test script test:ci never reaches, so the next orphaned suite gets caught instead of sitting there unexecuted. Wire it in as test-wiring:check, chained into test:ci. Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>
1 parent 07a43da commit 6fdbbc1

4 files changed

Lines changed: 244 additions & 2 deletions

File tree

.github/workflows/ci.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,9 @@ jobs:
889889
# matches the pattern already used by "Save Turborepo cache"/"Save TypeScript incremental build
890890
# cache" elsewhere in this file: still run after an earlier step's failure, just not after the job
891891
# was cancelled outright. The job as a whole still fails if any of the three fails.
892+
- name: UI tests (ui-kit)
893+
if: ${{ !cancelled() && (github.event_name == 'push' || needs.changes.outputs.ui == 'true') }}
894+
run: npm --workspace @loopover/ui-kit run test
892895
- name: UI tests (ui)
893896
if: ${{ !cancelled() && (github.event_name == 'push' || needs.changes.outputs.ui == 'true') }}
894897
run: npm --workspace @loopover/ui run test

package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
"db:migrations:immutable:check": "tsx scripts/check-released-migrations-immutable.ts",
6262
"turbo-inputs:check": "tsx scripts/check-turbo-typecheck-inputs.ts",
6363
"typecheck-coverage:check": "tsx scripts/check-typecheck-coverage.ts",
64+
"test-wiring:check": "tsx scripts/check-test-coverage-wiring.ts",
6465
"fixture-clock-races:check": "tsx scripts/check-fixture-clock-races.ts",
6566
"workspace-dep-ranges:check": "tsx scripts/check-workspace-dep-ranges.ts",
6667
"db:schema-drift:check": "tsx scripts/check-schema-drift.ts",
@@ -73,7 +74,7 @@
7374
"ui:lint": "npm run ui:kit:build && npm --workspace @loopover/ui-kit run format:check && npm --workspace @loopover/ui run format:check && npm --workspace @loopover/ui run lint && npm --workspace @loopover/ui-miner run format:check && npm --workspace @loopover/ui-miner run lint",
7475
"ui:typecheck": "npm run ui:kit:build && npm --workspace @loopover/ui-kit run typecheck && npm --workspace @loopover/ui run typecheck && npm --workspace @loopover/ui-miner run typecheck",
7576
"preui:test": "npm run check-node-version",
76-
"ui:test": "npm run ui:kit:build && npm --workspace @loopover/ui run test && npm --workspace @loopover/ui-miner run test",
77+
"ui:test": "npm run ui:kit:build && npm --workspace @loopover/ui-kit run test && npm --workspace @loopover/ui run test && npm --workspace @loopover/ui-miner run test",
7778
"ui:openapi": "tsx scripts/write-ui-openapi.ts",
7879
"ui:openapi:check": "tsx scripts/write-ui-openapi.ts --check",
7980
"control-plane:contract": "tsx scripts/gen-control-plane-contract.ts",
@@ -148,7 +149,7 @@
148149
"test:smoke:browser:install": "playwright install chromium",
149150
"test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts",
150151
"pretest:ci": "npm run check-node-version",
151-
"test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run fixture-clock-races:check && npm run typecheck-coverage:check && npm run checkers-wired:check && npm run focus-manifest-fields:check && npm run release-commit-parsing:check && npm run releasable-commit-types:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
152+
"test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run fixture-clock-races:check && npm run typecheck-coverage:check && npm run test-wiring:check && npm run checkers-wired:check && npm run focus-manifest-fields:check && npm run release-commit-parsing:check && npm run releasable-commit-types:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
152153
"test:release": "npm run test:ci && npm run changelog:check",
153154
"test:release:mcp": "npm run test:ci",
154155
"test:watch": "vitest",
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
#!/usr/bin/env node
2+
// `npm run test:ci` must actually RUN every workspace's declared `test` script (#10049).
3+
//
4+
// THE INCIDENT. `packages/loopover-ui-kit` ships `"test": "vitest run"` and a package-local vitest.config.ts
5+
// whose own header states the acceptance signal is "this suite runs and passes". Nothing invokes it: root
6+
// `ui:test` builds ui-kit but only ever runs @loopover/ui and @loopover/ui-miner's tests, `test:ci` reaches
7+
// ui-kit only through build/pack/lint/typecheck steps, and ci.yml has no ui-kit test step. The suite's 12
8+
// files (60+ tests) have sat there, never executed, since #7437/#7495 added them.
9+
//
10+
// That is the exact shape #9860 named: not a MISSING check, but a SILENTLY PARTIAL one -- trusted precisely
11+
// because everything around it (build, pack, lint, typecheck) is green. #9860's own fix for this class was
12+
// `scripts/check-typecheck-coverage.ts`, which proved `npm run typecheck` reaches every workspace that
13+
// declares one. This is that same computation for `test`.
14+
//
15+
// WHAT THIS COMPUTES. Every workspace under apps/*/packages/* that declares its own `test` script, and
16+
// whether the root `test:ci` chain reaches it -- following `npm run <script>` references transitively, so a
17+
// workspace covered through an intermediate script (`ui:test` -> `npm --workspace @loopover/ui-kit run
18+
// test`) counts as covered. Anything declaring a `test` nobody runs is reported.
19+
import { readFileSync, readdirSync } from "node:fs";
20+
import { join } from "node:path";
21+
import { fileURLToPath, URL } from "node:url";
22+
23+
export type TestWiringGap = { workspace: string; script: string };
24+
25+
export type WorkspaceWithTest = { name: string; dir: string };
26+
27+
/** Every `npm run <name>` this script body invokes (the root package's own scripts). */
28+
function referencedRootScripts(body: string): string[] {
29+
// `npm run x`, `npm run x --silent`, `npm --silent run x` -- all forms used in this package.json.
30+
return [...body.matchAll(/npm\s+(?:--\S+\s+)*run\s+([\w:.-]+)/g)].map((match) => match[1]).filter((name): name is string => Boolean(name));
31+
}
32+
33+
/** Every workspace whose OWN `test` this script body invokes directly. */
34+
function referencedWorkspaces(body: string): string[] {
35+
// `npm --workspace @scope/name run test` and `npm run test --workspace @scope/name`.
36+
const names = [
37+
...body.matchAll(/npm\s+--workspace[= ]\s*(\S+)\s+run\s+([\w:.-]+)/g),
38+
...body.matchAll(/npm\s+run\s+([\w:.-]+)\s+--workspace[= ]\s*(\S+)/g),
39+
];
40+
const out: string[] = [];
41+
for (const match of names) {
42+
// The two patterns capture (workspace, script) and (script, workspace) respectively; the workspace is
43+
// whichever capture looks like a package name.
44+
const [a, b] = [match[1], match[2]];
45+
const workspace = a?.startsWith("@") || a?.includes("/") ? a : b;
46+
const script = workspace === a ? b : a;
47+
if (workspace && script === "test") out.push(workspace);
48+
}
49+
return out;
50+
}
51+
52+
/**
53+
* PURE: workspaces that declare a `test` script the root `test:ci` chain never reaches.
54+
*
55+
* `scripts` is the root package's script map; `workspacesWithTest` is every workspace that declares one
56+
* (package name + directory). Reachability follows `npm run` references transitively from `entry`, because
57+
* a workspace is covered whether its `test` is invoked directly or through an intermediate script.
58+
*/
59+
export function findTestWiringGaps(
60+
scripts: Readonly<Record<string, string>>,
61+
workspacesWithTest: readonly WorkspaceWithTest[],
62+
entry = "test:ci",
63+
): TestWiringGap[] {
64+
const covered = new Set<string>();
65+
const seen = new Set<string>();
66+
const queue: string[] = [entry];
67+
while (queue.length > 0) {
68+
const name = queue.shift();
69+
if (!name || seen.has(name)) continue;
70+
seen.add(name);
71+
const body = scripts[name];
72+
if (body === undefined) continue;
73+
for (const workspace of referencedWorkspaces(body)) covered.add(workspace);
74+
queue.push(...referencedRootScripts(body));
75+
}
76+
return workspacesWithTest
77+
.filter(
78+
(workspace) =>
79+
!covered.has(workspace.name) &&
80+
!covered.has(workspace.name.replace(/^@[\w-]+\//, "")) &&
81+
!covered.has(workspace.dir),
82+
)
83+
.map((workspace) => ({ workspace: workspace.name, script: "test" }));
84+
}
85+
86+
/** Workspace package names (and their directories) that declare their own `test` script. */
87+
export function workspacesDeclaringTest(root: string): WorkspaceWithTest[] {
88+
const out: WorkspaceWithTest[] = [];
89+
for (const group of ["apps", "packages"]) {
90+
let dirs: string[];
91+
try {
92+
dirs = readdirSync(join(root, group), { withFileTypes: true }).filter((entry) => entry.isDirectory()).map((entry) => entry.name);
93+
} catch {
94+
continue;
95+
}
96+
for (const dir of dirs) {
97+
try {
98+
const manifest = JSON.parse(readFileSync(join(root, group, dir, "package.json"), "utf8")) as { name?: string; scripts?: Record<string, string> };
99+
if (manifest.name && manifest.scripts?.test) out.push({ name: manifest.name, dir: `${group}/${dir}` });
100+
} catch {
101+
// not a workspace package
102+
}
103+
}
104+
}
105+
return out;
106+
}
107+
108+
function main(): void {
109+
const root = join(fileURLToPath(new URL(".", import.meta.url)), "..");
110+
const rootManifest = JSON.parse(readFileSync(join(root, "package.json"), "utf8")) as { scripts?: Record<string, string> };
111+
const declared = workspacesDeclaringTest(root);
112+
const gaps = findTestWiringGaps(rootManifest.scripts ?? {}, declared);
113+
114+
if (gaps.length > 0) {
115+
console.error("`npm run test:ci` does not reach every workspace that declares a `test` script:\n");
116+
for (const gap of gaps) console.error(` ${gap.workspace} (declares "${gap.script}", never invoked)`);
117+
console.error(
118+
"\n A suite that exists, is written to pin real behaviour, and never runs is worse than no suite: it is\n" +
119+
" trusted BECAUSE nothing around it is red. #10049 found exactly this for @loopover/ui-kit's 12-file\n" +
120+
" vitest suite -- present since #7437/#7495, never executed.\n\n" +
121+
" Fix: chain the workspace's `test` into the root `test:ci` script (directly, or through one it\n" +
122+
" already calls, e.g. `ui:test`), so a declared test script always means a running test script.",
123+
);
124+
process.exit(1);
125+
}
126+
console.log(`test-wiring: OK — all ${declared.length} workspace test script(s) are reachable from \`npm run test:ci\`.`);
127+
}
128+
129+
if (process.argv[1]?.endsWith("check-test-coverage-wiring.ts")) main();
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
// The test-wiring checker must catch a workspace `test:ci` never reaches (#10049).
2+
//
3+
// The failure it guards is specific: a workspace declares its own `test` script and nothing in the root
4+
// `test:ci` chain ever invokes it, so the suite sits there -- written, reviewed, never executed -- while
5+
// everything ELSE (build, pack, lint, typecheck) stays green. #10049 found exactly this for
6+
// @loopover/ui-kit's 12-file vitest suite. So the positive case here is "a declared test nobody runs is
7+
// reported", and the negative cases pin every legitimate way a workspace can be covered, since a checker
8+
// that reports a covered workspace would be noise and would get muted.
9+
import { describe, expect, it } from "vitest";
10+
11+
import { findTestWiringGaps } from "../../scripts/check-test-coverage-wiring";
12+
13+
describe("findTestWiringGaps (#10049)", () => {
14+
it("REGRESSION: reports the exact #10049 shape — ui-kit's test script the root chain never invokes", () => {
15+
const scripts = {
16+
"test:ci": "npm run ui:lint && npm run ui:typecheck && npm run ui:test",
17+
"ui:lint": "npm run ui:kit:build && npm --workspace @loopover/ui-kit run format:check",
18+
"ui:typecheck": "npm run ui:kit:build && npm --workspace @loopover/ui-kit run typecheck",
19+
// Present in the repo, but only ever builds ui-kit -- never runs its test script.
20+
"ui:test": "npm run ui:kit:build && npm --workspace @loopover/ui run test && npm --workspace @loopover/ui-miner run test",
21+
"ui:kit:build": "npm run build --workspace @loopover/ui-kit",
22+
};
23+
expect(
24+
findTestWiringGaps(scripts, [
25+
{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" },
26+
{ name: "@loopover/ui", dir: "apps/loopover-ui" },
27+
]),
28+
).toEqual([{ workspace: "@loopover/ui-kit", script: "test" }]);
29+
});
30+
31+
it("counts a workspace reached THROUGH an intermediate script as covered", () => {
32+
// The fix chains ui-kit's test into ui:test; the workspace is then covered transitively.
33+
const scripts = {
34+
"test:ci": "npm run ui:test",
35+
"ui:test": "npm run ui:kit:build && npm --workspace @loopover/ui-kit run test && npm --workspace @loopover/ui run test",
36+
"ui:kit:build": "npm run build --workspace @loopover/ui-kit",
37+
};
38+
expect(findTestWiringGaps(scripts, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }])).toEqual([]);
39+
});
40+
41+
it("reports a workspace whose BUILD is invoked but whose test is not", () => {
42+
// A build may exercise some code as a side effect, but that is not a guarantee the test script runs.
43+
const scripts = { "test:ci": "npm run ui:kit:build", "ui:kit:build": "npm run build --workspace @loopover/ui-kit" };
44+
expect(findTestWiringGaps(scripts, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }])).toEqual([
45+
{ workspace: "@loopover/ui-kit", script: "test" },
46+
]);
47+
});
48+
49+
it("handles the reversed flag order, since both spellings appear in this package.json", () => {
50+
const scripts = { "test:ci": "npm run test --workspace @loopover/engine" };
51+
expect(findTestWiringGaps(scripts, [{ name: "@loopover/engine", dir: "packages/loopover-engine" }])).toEqual([]);
52+
});
53+
54+
it("does NOT count a non-test script invoked via --workspace as coverage", () => {
55+
const scripts = { "test:ci": "npm --workspace @loopover/ui-kit run build" };
56+
expect(findTestWiringGaps(scripts, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }])).toEqual([
57+
{ workspace: "@loopover/ui-kit", script: "test" },
58+
]);
59+
});
60+
61+
it("does NOT count a non-test script invoked via the reversed --workspace flag order", () => {
62+
const scripts = { "test:ci": "npm run build --workspace @loopover/ui-kit" };
63+
expect(findTestWiringGaps(scripts, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }])).toEqual([
64+
{ workspace: "@loopover/ui-kit", script: "test" },
65+
]);
66+
});
67+
68+
it("treats a workspace as covered when covered holds the unscoped name (scoped-name strip hit)", () => {
69+
const scripts = { "test:ci": "npm run test --workspace ui-kit" };
70+
expect(findTestWiringGaps(scripts, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }])).toEqual([]);
71+
});
72+
73+
it("does not treat a workspace as covered via scoped-name strip when the unscoped token is absent", () => {
74+
const scripts = { "test:ci": "echo noop" };
75+
expect(findTestWiringGaps(scripts, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }])).toEqual([
76+
{ workspace: "@loopover/ui-kit", script: "test" },
77+
]);
78+
});
79+
80+
it("terminates on a cyclic script graph instead of looping forever", () => {
81+
const scripts = { "test:ci": "npm run a", a: "npm run b", b: "npm run a" };
82+
expect(findTestWiringGaps(scripts, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }])).toEqual([
83+
{ workspace: "@loopover/ui-kit", script: "test" },
84+
]);
85+
});
86+
87+
it("reports nothing when no workspace declares a test at all", () => {
88+
expect(findTestWiringGaps({ "test:ci": "echo noop" }, [])).toEqual([]);
89+
});
90+
91+
it("tolerates a missing entry script rather than throwing", () => {
92+
// A renamed root script must fail loudly as a REPORT, not as a crash mid-CI.
93+
expect(findTestWiringGaps({}, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }])).toEqual([
94+
{ workspace: "@loopover/ui-kit", script: "test" },
95+
]);
96+
});
97+
98+
it("treats a workspace as covered when covered holds its directory path (npm --workspace <path> form)", () => {
99+
const scripts = { "test:ci": "npm --workspace packages/loopover-ui-kit run test" };
100+
expect(findTestWiringGaps(scripts, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }])).toEqual([]);
101+
});
102+
103+
it("accepts an explicit entry point other than the test:ci default", () => {
104+
const scripts = { "ui:test": "npm --workspace @loopover/ui-kit run test" };
105+
expect(
106+
findTestWiringGaps(scripts, [{ name: "@loopover/ui-kit", dir: "packages/loopover-ui-kit" }], "ui:test"),
107+
).toEqual([]);
108+
});
109+
});

0 commit comments

Comments
 (0)