Skip to content

Commit 3ae7c6f

Browse files
test(cli): split cli-bin into two files that share one build behind a lock directory (#26333)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi93kH3NmKTNuUFhcMmaWEDak)_ Fixes #25911 Slice 2 of #25911, after the two rail-label folds (#26255, #26309), and the card's last item. The StudioDesignSurface pilot landed in those two PRs. Of the two heaviest spawn files, `date-only-zone-pins-10183` was ruled out because its premise is stale (it is already one spawn, as recorded on #26255), and `cli-bin` is split here. `ui/packages/cli/src/__tests__/cli-bin.test.ts` is the second-heaviest file in the Test UI timings (74.1 s in `test-shard-timings.json`). It ran 48 serial spawns of the built CLI on one worker, after a `beforeAll` that builds `@object-ui/types` and `@object-ui/cli` whenever `dist/` is absent, which it is on every Test UI leg. It is now two files that share one build: - `cli-bin.surface.test.ts` holds the read-only half: package identity, `--version`, `--help`, every subcommand's `--help`, the `check` and `doctor` descriptions, and the documented flags. That is 24 tests. - `cli-bin.test.ts` keeps the commands that write, refuse or validate: `generate`'s and `analyze`'s retired flags, the retired commands, `validate` and `init`. That is 24 tests. - `cliBin.ts` is the new harness. It holds `CLI_BIN`, `PKG_PATH`, `run`, `tree` and `ensureCliBuilt`. `ensureCliBuilt` takes a lock before it checks `dist/` and builds. The lock is an `mkdirSync` directory under the OS temp dir, keyed by the package path. The second file to arrive waits for the first build, then finds `dist/` present, instead of building into the same directory at the same time. The holder writes its pid into the lock. A run killed while holding it (Ctrl-C, an OOM-killed worker) never reaches the `finally` that removes it. So a waiter that reads a dead holder pid (`process.kill(pid, 0)` throws ESRCH) removes the lock and takes it at once. A lock with no readable pid falls back to the age rule: one older than 10 minutes is taken over. A wait longer than 170 s throws with the lock path and the holder pid, inside the 180 s hook timeout. `ensureCliBuilt` takes an optional `lockDir` and `waitMs` so the tests can drive it. No test is deleted, and every assertion moves verbatim. Two tests are added for the lock (below). Both files keep the top-level `describe('@object-ui/cli bin')`, so every test's full name is unchanged. The mechanical check against the file on main gives the same `it`/`describe` titles, 41 of 41 as an identical sorted list, and the same `expect(` count, 89 against 89. The diff is test-only: no `src` file, no sdui record, no changeset. ## Before / after (from `ui/`, `--maxWorkers=2`, under `os-verify-lock`, shared-box seconds) Cold means `packages/{types,cli}/dist` and their `tsconfig.tsbuildinfo` removed first, which is the Test UI legs' state. Warm means `dist/` present. | | files | tests | cold wall | warm wall | |---|---|---|---|---| | before (main 3249e20) | 1 | 48 | 87.2 s (one worker: about 41 s build, then 46 s of spawns) | 45.6 s | | after (b810bfa) | 2 | 48 | 57.5 s (surface 55.0 s, commands 57.0 s) | 24.6 s (22.7 s, 24.1 s) | | after, with the two lock tests (bfd6005) | 2 | 50 | 53.4 s | 24.1 s | In the cold run each file spent about 34 s in its `beforeAll`: one built, and the other waited on the lock for that build. Each then ran its 24 spawns, about 22 s, side by side. The spawn CPU is the same 46 s before and after. The split moves it onto two workers. ⚠️ What the lock does not cover: it serialises builds on one machine. If the shard sequencer puts the two files on different Test UI legs, each leg builds once, about 35 s, so CI total CPU rises by one build while the heaviest file drops from about 87 s to about 57 s cold. On the same leg they share the build. The cost is stated here rather than hidden. ## The lock's liveness (added on review) `cli-bin.test.ts` ends with `the shared build lock (objectstack#25911)`. Both cases run after `beforeAll`, so `dist/` is present, and a build spawn that runs throws: - The first plants a lock holding the pid of a reaped child, and shows that `ensureCliBuilt` takes it over in under 5 s with `waitMs: 60_000`, then releases it. - The control plants a lock holding this process's own live pid. It shows the lock is waited on, times out at `waitMs: 600` naming that pid, and is not taken over. Ablation: with the pid check switched off, so only the age rule applies, the first case goes red after 60.2 s with `Timed out after 60000 ms … held by pid 15868`, and the control stays green. ## The spawned-build gate `scripts/__tests__/spawned-build-vitest-env-8598.test.ts` finds build spawns only in `*.test.ts` files, and resolves an `env:` identifier only within the spawning file. So the two `spawnSync(... 'build' ..., { env: BUILD_ENV })` calls stay in each test file, and are passed to `ensureCliBuilt` as closures. Each file declares its own `BUILD_ENV`. `cli-bin.test.ts` keeps the full objectui#8598 account, and the surface file points to it. The gate now counts 4 spawns in 2 files, all scrubbed, and passes its 4 tests. The gate's population comment ("Two build spawns exist today, both in `cli-bin.test.ts`") is updated to name both files. That is the PR's only edit outside `packages/cli`. The other comments that cite `cli-bin.test.ts` still hold, because that file still builds with tsup through `BUILD_ENV`: `check-comment-mask-corpus.mjs`, `check-hand-rolled-comment-mask.test.ts`, `zod-subpath-bundle-config-8598.test.ts`, `cli-cancel-and-nontty-8786.test.ts`, and `app-generator.test.ts` (the `init` pins stay in `cli-bin.test.ts`). ## Verification (head bfd6005) - `pnpm exec vitest run packages/cli/src/__tests__/cli-bin --maxWorkers=2`, cold then warm: 2 files, 50 tests passed each time. No lock directory is left under the temp dir afterwards. - `pnpm exec vitest run packages/cli scripts/__tests__/spawned-build-vitest-env-8598.test.ts --maxWorkers=2`: 32 files, 423 tests passed (31 cli files with 419 tests, plus the gate's 4). - `pnpm --filter @object-ui/cli type-check`, after `pnpm --filter '@object-ui/cli^...' build`: exit 0. `eslint` on the four changed files: 0 problems. - Derived gates (`node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`): 24 derived, 24 run, every one exit 0, and `--ran` reconciles as a derived zero. The first `check-comment-mask-corpus` run exited 1 with ENOENT: it walked `ui/.objectui-tmp/tsc-gate-*`, and a `packages/cli` vitest run I had going in the same checkout deleted that directory under it. That is objectui#11040's race, from my parallel run and not from this diff. Its re-run with nothing else running exited 0: 18,255 files, 0 disagree. The diff is 686 changed lines. Both heads were run: b810bfa and bfd6005, 24 of 24 at exit 0 each. --- _Generated by [Claude Code](https://claude.ai/code/session_012Vh1mJSJN3FeocpnWPNdH1)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_012Vh1mJSJN3FeocpnWPNdH1 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 40f5c6e commit 3ae7c6f

4 files changed

Lines changed: 466 additions & 220 deletions

File tree

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
/**
2+
* Smoke tests for the @object-ui/cli bin.
3+
*
4+
* These tests invoke the built `dist/cli.js` as a child process to verify
5+
* that the public CLI surface (commands, flags, exit codes) matches what
6+
* is documented in the README and content/docs/utilities/cli.mdx.
7+
*
8+
* If you change a flag or command name, update both this test file AND the
9+
* docs in the same PR (Rule #2: Documentation Driven Development).
10+
*
11+
* This file holds the read-only half: package identity, `--version`, `--help`
12+
* and the documented flags of every command. `cli-bin.test.ts` holds the
13+
* commands that write, refuse or validate. Both share `cliBin.ts`, which
14+
* builds `dist/` once behind a lock (objectstack#25911).
15+
*/
16+
import { describe, it, expect, beforeAll } from 'vitest';
17+
import { spawnSync } from 'node:child_process';
18+
import { readFileSync } from 'node:fs';
19+
20+
import { PKG_PATH, PKG_ROOT, REPO_ROOT, ensureCliBuilt, run } from './cliBin.js';
21+
22+
/**
23+
* The commands the CLI declares. The enumeration pin below holds this list
24+
* equal to the built CLI's `--help`, both directions (objectui#11496). `add`
25+
* (objectui#11496) and `create`, `lint`, `test` and `studio` (objectui#11500)
26+
* are deliberately absent: they were retired, and the bin refuses each as an
27+
* unknown command. The CLI docs give each of these a `### objectui NAME`
28+
* heading; keep them in step by hand, because nothing in this file reads the
29+
* docs.
30+
*/
31+
const SUBCOMMANDS = [
32+
'serve',
33+
'dev',
34+
'build',
35+
'start',
36+
'init',
37+
'generate',
38+
'doctor',
39+
'check',
40+
'validate',
41+
'analyze',
42+
] as const;
43+
44+
/**
45+
* The environment a build spawned by this file runs in: `VITEST` removed.
46+
* `cli-bin.test.ts` gives the full account (objectui#8598). It is declared
47+
* here, not imported, because the gate that holds every build spawn to it
48+
* resolves `env:` only within the spawning file (see `cliBin.ts`).
49+
*/
50+
const BUILD_ENV: NodeJS.ProcessEnv = (() => {
51+
const env = { ...process.env };
52+
delete env.VITEST;
53+
return env;
54+
})();
55+
56+
/**
57+
* The command names commander prints under `Commands:` in the root `--help`, in
58+
* order. An entry starts with exactly two spaces; a wrapped description line is
59+
* indented further, so it is not an entry. `generate|g` reads as `generate`.
60+
*/
61+
function helpCommandNames(helpText: string): string[] {
62+
const lines = helpText.split('\n');
63+
const start = lines.findIndex((line) => line.trim() === 'Commands:');
64+
if (start === -1) return [];
65+
const names: string[] = [];
66+
for (const line of lines.slice(start + 1)) {
67+
if (line.trim() === '') break;
68+
const entry = /^ {2}(\S+)/.exec(line);
69+
if (entry) names.push(entry[1].split('|')[0]);
70+
}
71+
return names;
72+
}
73+
74+
describe('@object-ui/cli bin', () => {
75+
beforeAll(() => {
76+
ensureCliBuilt({
77+
types: () =>
78+
spawnSync('pnpm', ['--filter', '@object-ui/types', 'run', 'build'], {
79+
cwd: REPO_ROOT,
80+
encoding: 'utf-8',
81+
stdio: 'pipe',
82+
env: BUILD_ENV,
83+
}),
84+
cli: () =>
85+
spawnSync('pnpm', ['run', 'build'], {
86+
cwd: PKG_ROOT,
87+
encoding: 'utf-8',
88+
stdio: 'pipe',
89+
env: BUILD_ENV,
90+
}),
91+
});
92+
}, 180_000);
93+
94+
describe('package metadata', () => {
95+
it('package.json declares the standalone @object-ui/cli identity', () => {
96+
const pkg = JSON.parse(readFileSync(PKG_PATH, 'utf-8'));
97+
expect(pkg.name).toBe('@object-ui/cli');
98+
expect(pkg.bin).toEqual({ objectui: './dist/cli.js' });
99+
expect(pkg.oclif).toBeUndefined();
100+
expect(pkg.dependencies?.['@oclif/core']).toBeUndefined();
101+
});
102+
});
103+
104+
describe('--version', () => {
105+
it('matches the version in package.json', () => {
106+
const pkg = JSON.parse(readFileSync(PKG_PATH, 'utf-8'));
107+
const res = run(['--version']);
108+
expect(res.code).toBe(0);
109+
expect(res.stdout.trim()).toBe(pkg.version);
110+
});
111+
});
112+
113+
// objectui#11496. One enumeration over the built CLI's commands, so the next
114+
// declared-but-unbuilt command cannot ship the way `add` did: a retired
115+
// command that comes back, or a new one this list does not name, turns this
116+
// red. It replaces a containment check that a retired command coming back
117+
// would have passed.
118+
describe('--help', () => {
119+
it('lists exactly the declared commands, no more and no fewer', () => {
120+
const res = run(['--help']);
121+
expect(res.code, res.stderr).toBe(0);
122+
const listed = helpCommandNames(res.stdout);
123+
// Lit control: commander lists its own `help` command last, so reading it
124+
// proves the parse ran to the end of the Commands section.
125+
expect(listed[listed.length - 1]).toBe('help');
126+
const declared = listed.filter((name) => name !== 'help');
127+
expect([...declared].sort()).toEqual([...SUBCOMMANDS].sort());
128+
expect(new Set(declared).size).toBe(declared.length);
129+
});
130+
131+
it('uses the "objectui" bin name (not "os ui")', () => {
132+
const res = run(['--help']);
133+
expect(res.stdout).toMatch(/Usage:\s+objectui/);
134+
expect(res.stdout).not.toContain('os ui');
135+
});
136+
});
137+
138+
describe.each(SUBCOMMANDS)('subcommand "%s"', (cmd) => {
139+
it('exits 0 on --help', () => {
140+
const res = run([cmd, '--help']);
141+
expect(res.code, res.stderr || res.stdout).toBe(0);
142+
expect(res.stdout).toContain(`objectui ${cmd}`);
143+
});
144+
});
145+
146+
// objectui#10524 — `check` is an advisory sweep, not a validator: a file whose
147+
// root carries a structural key is recognised by that key and never parsed,
148+
// and it exits non-zero only on unreadable JSON or on a `${…}` refused on a
149+
// text key its node never evaluates (objectui#4795). Its help line used to read
150+
// "Validate schema files". These assert the kind of claim and the command it
151+
// names, not the wording.
152+
describe('check --help does not present `check` as the validator (objectui#10524)', () => {
153+
const checkDescription = () => {
154+
const res = run(['check', '--help']);
155+
expect(res.code, res.stderr || res.stdout).toBe(0);
156+
// Commander prints `Usage: …`, a blank line, then the description paragraph.
157+
return res.stdout.split(/\n\s*\n/)[1] ?? '';
158+
};
159+
160+
it('does not describe `check` with the verb "validate"', () => {
161+
expect(checkDescription()).not.toMatch(/^\s*validates?\b/i);
162+
});
163+
164+
it('names `objectui validate` as the command that gives the verdict', () => {
165+
expect(checkDescription()).toContain('objectui validate');
166+
});
167+
});
168+
169+
// objectui#11500. `doctor` was described as "Diagnose and fix common issues",
170+
// and it fixes nothing: it reads the project and reports. Asserts the kind of
171+
// claim, not the wording.
172+
it('doctor --help does not claim to fix anything (objectui#11500)', () => {
173+
const res = run(['doctor', '--help']);
174+
expect(res.code, res.stderr || res.stdout).toBe(0);
175+
// Commander prints `Usage: …`, a blank line, then the description paragraph.
176+
const description = res.stdout.split(/\n\s*\n/)[1] ?? '';
177+
// Lit control: the paragraph that was read is not empty.
178+
expect(description.trim()).not.toBe('');
179+
expect(description).not.toMatch(/\bfix/i);
180+
});
181+
182+
describe('flag contracts (locked-in by docs)', () => {
183+
const cases: Array<[string, RegExp[]]> = [
184+
['dev', [/-p, --port <port>/, /-h, --host <host>/, /--no-open/]],
185+
['serve', [/-p, --port <port>/, /-h, --host <host>/, /--no-open/]],
186+
['build', [/-o, --out-dir <dir>/, /--clean/]],
187+
['start', [/-p, --port <port>/, /-h, --host <host>/, /-d, --dir <dir>/]],
188+
['init', [/-t, --template <template>/]],
189+
['generate', [/<type> <name>/]],
190+
['validate', [/\[schema\]/]],
191+
];
192+
it.each(cases)('%s exposes the documented flags', (cmd, patterns) => {
193+
const res = run([cmd, '--help']);
194+
expect(res.code).toBe(0);
195+
for (const re of patterns) {
196+
expect(res.stdout).toMatch(re);
197+
}
198+
});
199+
});
200+
});

0 commit comments

Comments
 (0)