test(cli): smoke-test the built CLI against a fixture scaffold - #178
Conversation
Two version bugs shipped because CI exercised library code and the packed install, but never ran the real CLI against a scaffold inside the suite. The smoke test spawns dist/cli.js in a temp project with a minimal .mex scaffold (ROUTER.md plus an AGENTS.md tool anchor, as setup would leave it) and asserts exit codes and output for the commands a user or agent reaches for first: --version, log, timeline (human and --json), check (human and --json), heartbeat, doctor, and --help. --version is asserted as strict equality against package.json, so a version that drifts from the published version fails the suite — verified by injecting a hard-coded literal into the bundle and watching the test catch it. Resolves mex-memory#57
|
@theDakshJaitly @theyashasvipandey — status: everything on this PR is done and green. Rebased-clean against current For reviewers: the suite spawns the built |
theDakshJaitly
left a comment
theDakshJaitly
left a comment
There was a problem hiding this comment.
Requesting three small test-harness fixes before merging: bound the child process, assert the JSON timeline exit status, and assert heartbeat's success output.
I tested the exact PR test file on main at 9648c9d. All seven smoke tests, the build, and typecheck passed; the smoke suite also passed alongside the existing CLI symlink invocation/build test. The wrong-version mutation was correctly detected. However, the heartbeat no-op and JSON timeline error-exit mutations both left all seven tests green, as detailed inline. A delayed child also demonstrated that Vitest's timeout does not interrupt spawnSync.
Local validation was on macOS; I did not rerun the full regression suite.
| const result = spawnSync(process.execPath, [cliPath, ...args], { | ||
| cwd: root, | ||
| encoding: "utf8", | ||
| env: { | ||
| ...process.env, | ||
| HOME: root, | ||
| MEX_TELEMETRY: "0", | ||
| DO_NOT_TRACK: "1", | ||
| NO_COLOR: "1", | ||
| }, | ||
| }); |
There was a problem hiding this comment.
[P2] Bound child-process execution
spawnSync has no timeout, so a CLI regression that never exits can block this worker beyond Vitest's test timeout. In a controlled probe, a three-second child delay with a one-second Vitest deadline reported failure only after the child returned (3.39 seconds). Please add a finite subprocess timeout and include result.error/signal and stderr in the failure diagnostic, so a hung command becomes an actionable test failure.
| const asJson = JSON.parse(mex(root, ["timeline", "--json"]).stdout) as { | ||
| events: Array<{ kind: string; message: string }>; | ||
| }; | ||
| expect(asJson.events).toEqual([ | ||
| expect.objectContaining({ kind: "decision", message: "smoke: chose the bounded resolver" }), | ||
| ]); |
There was a problem hiding this comment.
[P2] Assert the JSON timeline exit code
This parses stdout and discards the process status. Forcing only timeline --json to return exit code 1 while retaining its valid JSON output still passed all seven smoke tests. That misses the exit-code regression this suite is meant to catch under #57. Please retain the result and assert status === 0 before parsing stdout.
| const result = mex(root, ["heartbeat"]); | ||
| expect(result.status).toBe(0); |
There was a problem hiding this comment.
[P2] Assert heartbeat's success output
This only checks exit status. Replacing heartbeat with an immediate successful exit and no output still passed all seven smoke tests, so a missing or no-op command handler goes undetected. Please also assert that stdout contains HEARTBEAT_OK, matching #57's requirement to check both exit codes and key output.
…tput Review follow-ups on the CLI smoke suite: - spawnSync now passes a finite 10s timeout with SIGKILL, and any spawn error, kill signal, or missing exit status fails immediately with an actionable diagnostic (error message, signal, stderr) instead of blocking the worker or masking a hang. - timeline --json asserts exit status 0 before JSON.parse so a valid-JSON-but-exit-1 regression fails. - heartbeat asserts its HEARTBEAT_OK output so a silent no-op handler cannot pass on exit code alone. The describe block gets a 35s outer timeout (3 children x 10s plus fixture headroom) so the bounded subprocess timeout, not Vitest, is what ends a hung child.
|
Fixed the three findings in 089df37: a 10-second subprocess timeout with diagnostics, timeline JSON exit-status assertion, and HEARTBEAT_OK output assertion. Each regression mutation now fails the test. Final-head CI is green at 5f15d3d. @theDakshJaitly please re-review when you can. |
|
hi the review fixes are pushed in 089df37 every spawned child process is bounded with an explicit timeout and killed on expiry failures include error signal and stderr in the assertion messages exit status is asserted before json parsing and heartbeat output must contain the ok marker all ci checks are green could you take another look when you get time |
theDakshJaitly
left a comment
theDakshJaitly
left a comment
There was a problem hiding this comment.
Re-reviewed the final head: all three requested changes are resolved, with no remaining findings. Verified that a hanging child is killed after 10 seconds with useful diagnostics, a nonzero timeline JSON exit fails its status assertion, and a no-output heartbeat fails its output assertion. The wrong-version mutation is also detected. The seven smoke tests pass, and eight tests pass when combined with the existing CLI build/symlink check. Build, typecheck, and final-head CI are green.
2 participants
Resolves #57.
What
A smoke suite that spawns the built
dist/cli.jsin a temp project and asserts exit codes and output for the commands a user or agent reaches for first:--version— strict equality againstpackage.json's versionlog→timeline→timeline --json— one event rendered end to end in both output modescheckandcheck --json— healthy fixture reports100/100, JSON is parseable with the documented shapeheartbeat,doctor— run clean on the fixture--help— lists the top-level commandsFixture
Each test gets a fresh temp project with a minimal
.mex/scaffold:ROUTER.mdplus anAGENTS.mdtool anchor — the statemex setupleaves behind. The anchor matters: without it,checkreportsSCAFFOLD_ORPHANED(exit 1), which is correct product behavior and exactly the kind of drift the smoke test should be able to express, not fight.Why this would have caught the shipped bugs
The two version regressions shipped because CI exercised library code and the packed install, but never ran the real CLI against a scaffold inside the suite. I verified the detection property directly: injecting a hard-coded
VERSION = "0.7.0"into the bundle (reproducing the original hard-coded-literal bug) makes the--versiontest fail withexpected '0.7.0' to be '0.8.0'. CI already runsnpm run buildbeforenpm test, so the suite has the bundle it needs; the test also fails with an explicit message ifdist/cli.jsis missing.No production code is touched — test + fixture only.