refactor(test/cli): migrate harness short-lived path to AppProcess + FileSystem (#28366)

This commit is contained in:
Kit Langton
2026-05-19 16:18:19 -04:00
committed by GitHub
parent bc6c4c72ab
commit 80e5fb11c2
2 changed files with 64 additions and 26 deletions

View File

@@ -29,7 +29,10 @@ import { normalizeForSnapshot, PATH_SEP } from "../../lib/snapshot"
function normalize(text: string): string {
return normalizeForSnapshot(text, {
pathReplacements: [
[new RegExp(`<TMPDIR>${PATH_SEP}oc-cli-[a-z0-9]+`, "g"), "<HOME>"],
// Mixed-case [A-Za-z0-9] because node's mkdtemp suffix is mixed-case
// (the harness now uses FileSystem.makeTempDirectoryScoped under the
// hood). A `[a-z0-9]+` regex would leave uppercase chars trailing.
[new RegExp(`<TMPDIR>${PATH_SEP}oc-cli-[A-Za-z0-9]+`, "g"), "<HOME>"],
[/\s+\[string\] \[default: "<HOME>"\]/g, ' [string] [default: "<HOME>"]'],
],
})

View File

@@ -18,12 +18,12 @@
// without changing the fixture. Long-lived commands like `serve` will need a
// different return shape — see the TODO at the bottom of OpencodeCli.
import type { TestOptions } from "bun:test"
import { AppFileSystem } from "@opencode-ai/core/filesystem"
import { AppProcess } from "@opencode-ai/core/process"
import { Deferred, Duration, Effect, Layer, Queue, Scope, Stream } from "effect"
import { FetchHttpClient, HttpClient } from "effect/unstable/http"
import { ChildProcess } from "effect/unstable/process"
import path from "node:path"
import fs from "node:fs/promises"
import os from "node:os"
import { Process } from "@/util/process"
import { TestLLMServer } from "./llm-server"
import { testProviderConfig } from "./test-provider"
import { it } from "./effect"
@@ -182,33 +182,59 @@ export function withCliFixture<A, E>(
): Effect.Effect<A, E | unknown, Scope.Scope> {
return Effect.gen(function* () {
const llm = yield* TestLLMServer
const fs = yield* AppFileSystem.Service
const appProc = yield* AppProcess.Service
const home = path.join(os.tmpdir(), "oc-cli-" + Math.random().toString(36).slice(2))
yield* Effect.promise(() => fs.mkdir(home, { recursive: true }))
yield* Effect.addFinalizer(() =>
Effect.promise(() => fs.rm(home, { recursive: true, force: true }).catch(() => undefined)),
)
// FileSystem.makeTempDirectoryScoped handles both creation and scope-tied
// cleanup — replaces the old mkdir + addFinalizer pair.
const home = yield* fs.makeTempDirectoryScoped({ prefix: "oc-cli-" })
const configJson = JSON.stringify(testProviderConfig(llm.url))
const env = isolatedEnv(home, configJson)
const spawn = (args: string[], opts?: SpawnOpts): Effect.Effect<RunResult> =>
Effect.promise(async () => {
const start = Date.now()
// Process.run pipes stdout/stderr by default and returns them as Buffers.
const result = await Process.run(["bun", "run", "--conditions=browser", cliEntry, ...args], {
cwd: home,
timeout: opts?.timeoutMs ?? 30_000,
env: { ...process.env, ...env, ...opts?.env },
nothrow: true,
})
return {
exitCode: result.code,
stdout: result.stdout.toString(),
stderr: result.stderr.toString(),
durationMs: Date.now() - start,
}
const spawn = Effect.fn("opencode.spawn")(function* (args: string[], opts?: SpawnOpts) {
const start = Date.now()
const timeoutMs = opts?.timeoutMs ?? 30_000
// stdin: "ignore" so the child doesn't see a piped stdin and block
// on `Bun.stdin.text()` (see src/cli/cmd/run.ts — non-TTY stdin is
// consumed as the prompt). The old Process.run wrapper defaulted to
// ignore; ChildProcess.make defaults to pipe, so we set it explicitly.
const command = ChildProcess.make("bun", ["run", "--conditions=browser", cliEntry, ...args], {
cwd: home,
env: { ...env, ...opts?.env },
extendEnv: true,
stdin: "ignore",
})
// Pass timeout to appProc.run rather than wrapping with
// Effect.timeoutOrElse externally: AppProcess.run is itself scoped, so
// its built-in timeout triggers the acquireRelease kill finalizer
// inside cross-spawn-spawner *before* surfacing the AppProcessError —
// guaranteeing the child is dead by the time the test continues.
// External timeoutOrElse interrupts the run fiber but races the
// scope close, which can leak the child past the test boundary.
//
// Catch AppProcessError (timeout OR spawn failure) and synthesize a
// non-zero result so the test sees it via the usual `expectExit`
// path rather than as an unhandled Effect failure.
const result = yield* appProc.run(command, { timeout: Duration.millis(timeoutMs) }).pipe(
Effect.catchTag("AppProcessError", (err) =>
Effect.succeed({
command: err.command,
exitCode: err.exitCode ?? -1,
stdout: Buffer.alloc(0),
stderr: Buffer.from((err.stderr ?? String(err.cause ?? err.message)) + "\n"),
stdoutTruncated: false,
stderrTruncated: false,
} satisfies AppProcess.RunResult),
),
)
return {
exitCode: result.exitCode,
stdout: result.stdout.toString(),
stderr: result.stderr.toString(),
durationMs: Date.now() - start,
}
})
const run = (message: string, opts?: RunOpts): Effect.Effect<RunResult> => {
const argv: string[] = ["run"]
@@ -380,7 +406,16 @@ export function withCliFixture<A, E>(
return yield* fn({ llm, home, opencode })
// FetchHttpClient is provided so test bodies can `yield* HttpClient.HttpClient`
// and hit endpoints on `opencode.serve()` without rolling their own fetch.
}).pipe(Effect.provide(Layer.mergeAll(TestLLMServer.layer, FetchHttpClient.layer)))
}).pipe(
Effect.provide(
Layer.mergeAll(
TestLLMServer.layer,
FetchHttpClient.layer,
AppFileSystem.defaultLayer,
AppProcess.defaultLayer,
),
),
)
}
function parseJsonEvents(stdout: string): Array<Record<string, unknown>> {