test(headless): run ordinary CLI command semantics in process - #2480
Conversation
Part of apache#2387. The Headless CLI and contamination-scan suites started a Node subprocess per assertion for scenarios that only exercise argument validation, task-run business logic, and report verdicts — semantics the exported entry points already expose. - cli.test.ts: 10 of 11 tests now run through mapLegacyMakaHeadlessArgs + runMakaEvalCli with stdout/stderr captured at the process-stream seam and env overrides applied and restored around the call. The non-Headless-root test keeps the real bin route as the representative wiring contract (real exit code, stack-free stderr). - contamination-scan-cli.test.ts: 13 of 14 tests call the script's exported main(argv), mirroring the executable footer exactly (thrown error -> stderr + exit 2). The no-argument rejection keeps the real subprocess as representative coverage of that footer itself, including its realpath main-module guard. - New shared helper withCapturedProcessIo swaps and restores the process-wide stream writers; safe for the sequential node:test runs these files use. runtime-policy-ab-cli.test.ts is intentionally untouched: its single test is the representative subprocess for run-runtime-policy-ab.mjs, whose main() is not exported, and adding an export to shave 0.6s is not warranted. (harness-ab-cli.test.ts was part of this change until apache#2462 deleted that suite on main.) Timing (node --test, local, warm build): cli.test.js 21.8s -> 13.1s (spawns 20 -> 2) contamination-scan-cli.test.js 1.05s -> 0.63s (spawns 14 -> 1) Tests pass across 3 consecutive rounds.
6bfc7b3 to
e424d56
Compare
|
Thanks — the boundary split here looks good. Ordinary Headless CLI semantics now use the existing exported entry points, while executable/footer behavior retains representative subprocess coverage. The shared IO helper is small enough as-is, and the heavy-task journey still protects a distinct two-invocation trace contract. One non-blocking P3 suggestion: The contamination-scan subprocess coverage now keeps only the no-argument Keeping one “retrieval signal detected → real subprocess exit 1 + stdout report” case would preserve that fail-closed executable contract. If spawn count matters, one of the two similar Headless CLI error-process launches could be moved in process to offset it. This does not need to block the PR. The current implementation and CI look sufficient to merge. Thanks! |
Part of #2387 (Headless workspace; the CLI workspace is #2476 per the issue's separate-PR requirement).
Problem
The Headless suites started a Node subprocess per assertion for scenarios that only exercise argument validation, task-run business logic, and report verdicts.
cli.test.tswas the sharpest case: 11 tests, 20 subprocess launches, 21.8s — each launch paying the full module-graph load of the headless CLI to test semantics thatrunMakaEvalClialready exposes as an exportedPromise<number>router.Change
No production changes. Every conversion goes through an already-exported entry:
cli.test.ts— 10 of 11 tests run throughmapLegacyMakaHeadlessArgs+runMakaEvalCli, the same mapping and router the bin runs, with stdout/stderr captured at the process-stream seam and env overrides applied/restored around the call. The non-Headless-root test keeps the real bin route as the representative wiring contract (real exit code, stack-free stderr hygiene).contamination-scan-cli.test.ts— 13 of 14 tests call the script's exportedmain(argv), with the executable footer's contract mirrored exactly (thrown error → stderr + exit 2, verdict codes returned). The no-argument rejection keeps the real subprocess as representative coverage of the footer itself, including its realpath main-module guard. The--markdowntest gains an explicitassert.equal(code, 0)that was previously implicit inexecFileAsyncnot rejecting.withCapturedProcessIoswaps and restores the process-wide stream writers; safe for the sequentialnode:testexecution these files use.An earlier revision of this branch also converted three pre-launch validation tests in
harness-ab-cli.test.ts; #2462 deleted that suite on main, so that part is gone after rebase.Deliberately not converted
runtime-policy-ab-cli.test.ts: its single test is the representative subprocess forrun-runtime-policy-ab.mjs, whosemain()is not exported — adding an export to shave 0.6s is a production touch the saving does not justify.pi-cli-json-transport.test.tsalready uses an injected fake child;task-agent-controller/harbor-cellsubprocesses are the behavior under test.Retained process contracts
Timing
node --test, same machine, warm build:The residual 13s in
cli.test.jsis dominated by one heavy-task journey (7.4s) whose cost is task-run business logic, not startup tax — visible now that the tax is gone.Tests pass across 3 consecutive rounds after the rebase onto #2462/#2473; typecheck, biome, and knip are clean (knip delta against baseline:
run-contamination-scan.mjsmoves from unused to used).Supersedes #2479: that branch predated the #2462 merge and conflicted with it, so GitHub could not build the PR merge commit and never created check suites — which also explains the "CI never triggered" mystery there (my platform-failure guess in its closing comment was wrong).
cc @Astro-Han