refactor(test/cli): simplify pass on tier-A + prebuild
Applies findings from the third simplify pass: 1. \`cliArgv\` is a module-level const, not a function — prebuiltCli is read once at module init and never mutated, so the per-spawn function allocation was pure overhead. 2. Help-snapshot failures surface via \`Effect.fail\` instead of \`throw\`, symmetric with the \`Effect.fail\` already inside the partition above. Keeps the failure typed in the Effect channel rather than as a defect. 3. \`prebuild-test-cli.ts\` skips the build when the binary is already newer than every file in src/ — saves the 2.8s rebuild cost on every subsequent invocation. Pass --force to bypass. 4. \`prebuild-test-cli.ts\` verifies the built binary is executable before symlinking — catches a silently-failed build leaving stale output instead of letting tests fail with confusing exec errors later. Verified: 331/331 CLI tests pass; typecheck clean; skip-if-fresh + --force behave as documented.
This commit is contained in:
Kit Langton
@@ -32,16 +32,58 @@ const arch = process.arch === "x64" ? "x64" : process.arch === "arm64" ? "arm64"
|
||||
const targetDir = path.join(dir, "dist", `opencode-${platform}-${arch}`)
|
||||
const binaryName = process.platform === "win32" ? "opencode.exe" : "opencode"
|
||||
const builtBinary = path.join(targetDir, "bin", binaryName)
|
||||
|
||||
// Stable path the harness reads via OPENCODE_TEST_CLI_PATH. Symlinked so
|
||||
// the binary itself remains the platform-specific one (build.ts manages it).
|
||||
const stableBinary = path.join(dir, "dist", "test-cli", "bin", binaryName)
|
||||
|
||||
console.log(`Building test CLI binary for ${platform}-${arch}...`)
|
||||
const start = Date.now()
|
||||
await $`bun script/build.ts --single --skip-embed-web-ui --skip-install`
|
||||
const buildMs = Date.now() - start
|
||||
console.log(`Build complete in ${buildMs}ms: ${builtBinary}`)
|
||||
const force = process.argv.includes("--force")
|
||||
|
||||
// Walk src/ and return the newest mtime seen. Faster than `git status` for
|
||||
// the freshness check and works for uncommitted edits. Returns 0 on error
|
||||
// so a missing src/ tree forces a rebuild via the comparison below.
|
||||
async function newestMtimeMs(root: string): Promise<number> {
|
||||
let max = 0
|
||||
async function walk(p: string) {
|
||||
let entries: { name: string; isDirectory: () => boolean; isFile: () => boolean }[]
|
||||
try {
|
||||
entries = await fs.readdir(p, { withFileTypes: true })
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
for (const entry of entries) {
|
||||
const full = path.join(p, entry.name)
|
||||
if (entry.isDirectory()) {
|
||||
if (entry.name === "node_modules" || entry.name === "dist") continue
|
||||
await walk(full)
|
||||
} else if (entry.isFile()) {
|
||||
const stat = await fs.stat(full).catch(() => null)
|
||||
if (stat && stat.mtimeMs > max) max = stat.mtimeMs
|
||||
}
|
||||
}
|
||||
}
|
||||
await walk(root)
|
||||
return max
|
||||
}
|
||||
|
||||
async function fresh(): Promise<boolean> {
|
||||
const binStat = await fs.stat(builtBinary).catch(() => null)
|
||||
if (!binStat) return false
|
||||
const srcMs = await newestMtimeMs(path.join(dir, "src"))
|
||||
return binStat.mtimeMs > srcMs
|
||||
}
|
||||
|
||||
if (!force && (await fresh())) {
|
||||
console.log(`Test CLI binary is up to date: ${builtBinary}`)
|
||||
} else {
|
||||
console.log(`Building test CLI binary for ${platform}-${arch}...`)
|
||||
const start = Date.now()
|
||||
await $`bun script/build.ts --single --skip-embed-web-ui --skip-install`
|
||||
console.log(`Build complete in ${Date.now() - start}ms: ${builtBinary}`)
|
||||
}
|
||||
|
||||
// Verify the binary exists and is executable before symlinking — catches
|
||||
// a silently-failed build that left a stale or partial output behind.
|
||||
await fs.access(builtBinary, fs.constants.X_OK).catch(() => {
|
||||
throw new Error(`Built binary missing or not executable: ${builtBinary}`)
|
||||
})
|
||||
|
||||
await fs.mkdir(path.dirname(stableBinary), { recursive: true })
|
||||
await fs.rm(stableBinary, { force: true })
|
||||
|
||||
@@ -121,7 +121,9 @@ describe("opencode CLI help-text snapshots", () => {
|
||||
expect(normalize(result.stderr)).toMatchSnapshot(`opencode ${argv.join(" ")} --help`)
|
||||
}
|
||||
if (failures.length > 0) {
|
||||
throw new Error(`Help text failed for:\n ${failures.join("\n ")}`)
|
||||
// Keep the failure in the Effect channel — symmetric with the
|
||||
// Effect.fail inside the partition above, not a defect.
|
||||
yield* Effect.fail(new Error(`Help text failed for:\n ${failures.join("\n ")}`))
|
||||
}
|
||||
}),
|
||||
180_000,
|
||||
|
||||
@@ -31,18 +31,15 @@ import { it } from "./effect"
|
||||
const opencodeRoot = path.resolve(import.meta.dir, "../../")
|
||||
const cliEntry = path.join(opencodeRoot, "src/index.ts")
|
||||
|
||||
// Opt-in pre-built binary path. If set, subprocess tests spawn the binary
|
||||
// directly instead of `bun run src/index.ts`, skipping JIT + plugin init
|
||||
// (~3x speedup on isolation-env spawns that hit DB migration). Produced by
|
||||
// `bun script/prebuild-test-cli.ts`; the harness silently falls back to dev
|
||||
// mode if the var is unset, so this is strictly opt-in.
|
||||
// Argv prefix for spawning the CLI. If OPENCODE_TEST_CLI_PATH is set,
|
||||
// subprocess tests spawn the pre-built binary directly (~3x speedup on
|
||||
// isolation-env spawns that hit DB migration; produced by
|
||||
// `bun script/prebuild-test-cli.ts`). Otherwise falls back to dev mode —
|
||||
// strictly opt-in, default behavior unchanged.
|
||||
const prebuiltCli = process.env["OPENCODE_TEST_CLI_PATH"]
|
||||
|
||||
// Argv prefix for spawning the CLI. Either the pre-built binary (single
|
||||
// argument) or `bun run --conditions=browser src/index.ts` (four arguments).
|
||||
function cliArgv(): string[] {
|
||||
return prebuiltCli ? [prebuiltCli] : ["bun", "run", "--conditions=browser", cliEntry]
|
||||
}
|
||||
const cliArgv: readonly string[] = prebuiltCli
|
||||
? [prebuiltCli]
|
||||
: ["bun", "run", "--conditions=browser", cliEntry]
|
||||
|
||||
export const testModelID = "test/test-model"
|
||||
|
||||
@@ -209,7 +206,7 @@ export function withCliFixture<A, E>(
|
||||
Effect.promise(async () => {
|
||||
const start = Date.now()
|
||||
// Process.run pipes stdout/stderr by default and returns them as Buffers.
|
||||
const result = await Process.run([...cliArgv(), ...args], {
|
||||
const result = await Process.run([...cliArgv, ...args], {
|
||||
cwd: home,
|
||||
timeout: opts?.timeoutMs ?? 30_000,
|
||||
env: { ...process.env, ...env, ...opts?.env },
|
||||
@@ -248,7 +245,7 @@ export function withCliFixture<A, E>(
|
||||
// as a finalizer error during test teardown.
|
||||
const proc = yield* Effect.acquireRelease(
|
||||
Effect.sync(() =>
|
||||
Bun.spawn([...cliArgv(), ...argv], {
|
||||
Bun.spawn([...cliArgv, ...argv], {
|
||||
cwd: home,
|
||||
env: { ...process.env, ...env, ...opts?.env },
|
||||
stdout: "pipe",
|
||||
@@ -319,7 +316,7 @@ export function withCliFixture<A, E>(
|
||||
// Either way we await proc.exited so the test scope doesn't leak.
|
||||
const proc = yield* Effect.acquireRelease(
|
||||
Effect.sync(() =>
|
||||
Bun.spawn([...cliArgv(), ...argv], {
|
||||
Bun.spawn([...cliArgv, ...argv], {
|
||||
cwd: opts?.cwd ?? home,
|
||||
env: { ...process.env, ...env, ...opts?.env },
|
||||
stdin: "pipe",
|
||||
|
||||
Reference in New Issue
Block a user