refactor(test/lib): generalize run-process harness into cli-process#28253
Merged
Conversation
The run-process harness was named for the one command it covered, but the fixture (env isolation + tmpdir + TestLLMServer + subprocess spawn) is generic — it can host tests for any opencode subcommand. Renaming makes the surface match the eventual scope. Mechanical: - test/lib/run-process.ts → test/lib/cli-process.ts (git rename preserves history) - withRunFixture → withCliFixture - RunFixture → CliFixture - runIt → cliIt - The one consumer (test/cli/run/run-process.test.ts) updated to match. Kept as-is because they're still command-specific or still useful: - OpencodeCli (the interface) - RunOpts / RunResult (specific to opencode.run, but RunResult is the shape every spawn returns) - opencode.run / opencode.spawn / opencode.expectExit / opencode.parseJsonEvents A TODO inside OpencodeCli documents the next step: adding long-lived builders (opencode.serve, opencode.acp) needs a different return shape than the short-lived run/spawn — they need to yield a handle with .url / .kill that lives in the surrounding Scope. Will be added when the first long-lived command gets test coverage. Motivation: a future Effect CLI migration needs behavioral-parity tests across the full CLI surface. This rename sets up the harness to host that coverage without a rename in every follow-up PR. Verified: bun typecheck clean, 4/4 existing tests pass in ~10s.
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
1 participant
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.
Quick rename PR. The run-process harness landed in #28230 covering only the
runcommand, but the fixture itself (env isolation + tmpdir + TestLLMServer + subprocess spawn) is CLI-generic. Renaming makes the file name match the eventual scope and sets up follow-ups (smoke tests across the CLI surface) to land without a rename round.Changes
test/lib/run-process.ts→test/lib/cli-process.ts(git rename preserves history)withRunFixture→withCliFixtureRunFixture→CliFixturerunIt→cliIttest/cli/run/run-process.test.ts) updated to matchKept the names that are still useful:
OpencodeCliinterfaceRunOpts/RunResulttypes (RunResultis the shape every spawn returns;RunOptsis specific to therunbuilder)opencode.run/opencode.spawn/opencode.expectExit/opencode.parseJsonEventsWhat this enables
A TODO inside
OpencodeClidocuments the next addition: long-lived builders forserve/acpwill need a return-handle shape (yielding.url/.killfrom aScope). Those land per-command as they're needed.Motivation
A future Effect CLI migration needs behavioral-parity tests across the full CLI surface. This rename is the cheapest possible step toward that — the harness is ready to host coverage for any subcommand without per-PR renaming churn.
Verified
bun typecheck— cleanbun run test test/cli/run/run-process.test.ts— 4/4 green in ~10sNet diff
Mechanical: 1 file renamed, 1 consumer updated, ~10 lines changed inside the renamed file.