test(cli): help-text snapshots for every CLI command#28267
Open
kitlangton wants to merge 3 commits into
Open
Conversation
e38f49f to
8513472
Compare
One test file. Spawns `opencode <cmd> --help` for every documented command + key subcommand (35 in total) in parallel under concurrency:8, snapshots the stderr output (yargs writes --help to stderr, not stdout). Snapshots are normalized — the tmpdir prefix that bleeds through `acp --cwd`'s default is rewritten to `<HOME>` so test runs in different sandboxes stay stable. macOS `/private` realpath form and the unresolved `os.tmpdir()` form both covered. Pinned snapshots catch flag removals, renames, reordering, and exit-code regressions across the entire user-facing CLI surface in one place. Diff in the .snap file is the surface-change report. Excluded: `opencode completion --help` is a yargs built-in that emits top-level help and exits 1; not a real opencode command. ~6s wall-clock thanks to parallel spawns.
Applied review findings from the simplify pass:
1. Extract fromBunStream(name, get) and forkStderrDrain(stream, into)
helpers — 4 duplicated Stream.fromReadableStream call sites collapse
to one factory, and the identical stderr drain across serve/acp is
now a single helper. Error messages now include the underlying cause
instead of swallowing it.
2. acp.send awaits proc.stdin.write's backpressure promise. The bare
Effect.sync was discarding the Promise<number> form, which can
reorder ndjson lines under pipe-buffer-full conditions and corrupt
framing.
3. acp.close drops the try/catch around proc.stdin.end() — idempotent
in Bun, the bare catch only masked future regressions.
4. Effect.ignore on stream drains is now Effect.ignore({ log: true })
so a real protocol or decode error surfaces in test debug output
instead of disappearing silently.
5. Help-snapshots replaces the manual failures[] accumulator + continue
with Effect.partition. Same behavior, no mutable state, declarative.
324/324 CLI tests stay green; typecheck clean.
2083150 to
13e68a5
Compare
3 tasks
yargs wraps the \`[string] [default: "..."]\` clause based on the pre-normalized default value's character length, so a different random tmpdir width produces a different leading-whitespace count on the wrapped continuation line. After normalizing the path to \`<HOME>\` we were left with a one-space drift between runs. Collapse the wrap-dependent whitespace immediately before the clause so the snapshot is byte-stable regardless of home path length. Verified by deleting the .snap and regenerating across 3 runs.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
One test file. Spawns `opencode --help` for every documented command + key subcommand (35 total) in parallel under `concurrency: 8`, snapshots the stderr output (yargs writes `--help` to stderr, not stdout).
Pinned snapshots catch flag removals, renames, reordering, and exit-code regressions across the entire user-facing CLI surface in one place. The diff in the .snap file is the surface-change report.
Why this matters
This is the broad-coverage layer that makes the future Effect CLI migration (yargs → effect-smol/cli) safe to attempt: if a refactor preserves the surface, snapshots stay green; if it doesn't, the diff names exactly which command(s) changed.
Implementation notes
Stacked on #28263 + #28265
Branches off the acp-builder branch. Once both upstream PRs merge, the diff here shrinks to just the snapshot test + .snap file.
Test plan
Stack