From ef9b9fb6bf93fdcf653973c931f00fe179efd474 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 27 Aug 2026 10:55:20 -0400 Subject: [PATCH] test(client): scope service subprocess fixtures per test (#45471) --- .../client/test/fixture/service-fixture.ts | 56 +++++ .../client/test/fixture/service-timing.ts | 2 +- packages/client/test/promise-service.test.ts | 130 ++++------ packages/client/test/service.test.ts | 235 +++++++----------- 4 files changed, 197 insertions(+), 226 deletions(-) create mode 100644 packages/client/test/fixture/service-fixture.ts diff --git a/packages/client/test/fixture/service-fixture.ts b/packages/client/test/fixture/service-fixture.ts new file mode 100644 index 00000000000..a8dac655d77 --- /dev/null +++ b/packages/client/test/fixture/service-fixture.ts @@ -0,0 +1,56 @@ +import { mkdtemp, rm } from "node:fs/promises" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { waitForExit } from "./service-timing" + +export async function serviceFixture() { + const directory = await mkdtemp(join(tmpdir(), "opencode-client-service-")) + const registration = join(directory, "service.json") + const processes: Bun.Subprocess[] = [] + const pids = new Set() + const command = (mode: string, ...args: string[]) => [ + process.execPath, + join(import.meta.dir, "service.ts"), + registration, + mode, + ...args, + ] + + return { + directory, + registration, + command, + spawn(mode: string, ...args: string[]) { + const subprocess = Bun.spawn(command(mode, ...args), { stdout: "ignore", stderr: "inherit" }) + processes.push(subprocess) + return subprocess + }, + // Service.ensure detaches contenders; track the elected process before asserting. + track(pid: number) { + pids.add(pid) + }, + async waitForFile(file = registration) { + for (let attempt = 0; attempt < 600; attempt++) { + if (await Bun.file(file).exists()) return + await Bun.sleep(5) + } + throw new Error(`Timed out waiting for ${file}`) + }, + async [Symbol.asyncDispose]() { + await Promise.all([ + ...processes.map(async (subprocess) => { + subprocess.kill("SIGTERM") + await subprocess.exited + }), + ...[...pids].map(async (pid) => { + try { + process.kill(pid, "SIGTERM") + } catch (error) { + if (!(error instanceof Error && "code" in error && error.code === "ESRCH")) throw error + } + await waitForExit(pid) + }), + ]).finally(() => rm(directory, { recursive: true, force: true })) + }, + } +} diff --git a/packages/client/test/fixture/service-timing.ts b/packages/client/test/fixture/service-timing.ts index 362ce6869bc..f4db65ae1f5 100644 --- a/packages/client/test/fixture/service-timing.ts +++ b/packages/client/test/fixture/service-timing.ts @@ -10,7 +10,7 @@ const timing = { stopPollInterval: 5, } -export function accelerate(ensure: (options: A) => B) { +export function accelerate(ensure: (options?: A) => B) { return (options: A) => ensure(withEnsureTiming(options, timing)) } diff --git a/packages/client/test/promise-service.test.ts b/packages/client/test/promise-service.test.ts index 2f344015c8c..740dd1b1172 100644 --- a/packages/client/test/promise-service.test.ts +++ b/packages/client/test/promise-service.test.ts @@ -1,23 +1,15 @@ -import { afterEach, expect, test } from "bun:test" -import { mkdtemp, rm } from "node:fs/promises" -import { tmpdir } from "node:os" -import { join } from "node:path" +import { expect, test } from "bun:test" import { Service, type EnsureReason } from "../src/promise/service" -import { accelerate, waitForExit } from "./fixture/service-timing" +import { serviceFixture } from "./fixture/service-fixture" +import { accelerate } from "./fixture/service-timing" -const fixture = join(import.meta.dir, "fixture/service.ts") const ensure = accelerate(Service.ensure) -const processes: Bun.Subprocess[] = [] -const directories: string[] = [] - -afterEach(async () => { - processes.forEach((process) => process.kill("SIGTERM")) - await Promise.all(processes.splice(0).map((process) => process.exited)) - await Promise.all(directories.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))) -}) test("discovers a registered service", async () => { - const registration = await setup("graceful") + await using fixture = await serviceFixture() + const registration = fixture.registration + fixture.spawn("graceful") + await fixture.waitForFile() expect(await Service.discover({ file: registration, version: "test" })).toEqual( expect.objectContaining({ url: expect.stringMatching(/^http:\/\//) }), @@ -26,7 +18,10 @@ test("discovers a registered service", async () => { }) test("discovers a compatible registered service", async () => { - const registration = await setup("compatible") + await using fixture = await serviceFixture() + const registration = fixture.registration + fixture.spawn("compatible") + await fixture.waitForFile() expect(await Service.discover({ file: registration, version: "2.1.0" })).toBeUndefined() expect(await Service.discover({ file: registration, version: "2.1.0-next.1" })).toEqual( @@ -39,66 +34,59 @@ test("discovers a compatible registered service", async () => { }) test("ensures a missing service with native promises", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration const starts: EnsureReason[] = [] const endpoint = await ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "coordinated"], + command: fixture.command("coordinated"), onStart: (reason) => starts.push(reason), }) const info = await Bun.file(registration).json() - try { - expect(endpoint.url).toBe(info.url) - expect(starts).toEqual(["missing"]) - } finally { - process.kill(info.pid, "SIGTERM") - await waitForExit(info.pid) - } + fixture.track(info.pid) + + expect(endpoint.url).toBe(info.url) + expect(starts).toEqual(["missing"]) }) test("adds configured environment variables with native promises", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration const endpoint = await ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "environment"], + command: fixture.command("environment"), env: { OPENCODE_SERVICE_ENV_TEST: "configured" }, }) const info = await Bun.file(registration).json() + fixture.track(info.pid) - try { - expect(endpoint.url).toBe(info.url) - expect(await Bun.file(registration + ".environment").text()).toBe("configured") - } finally { - process.kill(info.pid, "SIGTERM") - await waitForExit(info.pid) - } + expect(endpoint.url).toBe(info.url) + expect(await Bun.file(registration + ".environment").text()).toBe("configured") }) test("waits for a live contender when another native contender fails", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration const endpoint = await ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "coordinated-failed-loser", "300"], + command: fixture.command("coordinated-failed-loser", "300"), }) const info = await Bun.file(registration).json() - try { - expect(endpoint.url).toBe(info.url) - } finally { - process.kill(info.pid, "SIGTERM") - await waitForExit(info.pid) - } + fixture.track(info.pid) + + expect(endpoint.url).toBe(info.url) }) test("reports a failed registered service", async () => { - const registration = await setup("failed-owner") + await using fixture = await serviceFixture() + const registration = fixture.registration + fixture.spawn("failed-owner") + await fixture.waitForFile() await expect(ensure({ file: registration, version: "test", command: [] })).rejects.toThrow( "Background service failed to start", @@ -106,12 +94,12 @@ test("reports a failed registered service", async () => { }) test("reports a bounded contender stderr tail with native promises", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration const error = await Service.ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "stderr-failed"], + command: fixture.command("stderr-failed"), }).catch((error: unknown) => error) expect(error).toBeInstanceOf(Error) @@ -121,58 +109,34 @@ test("reports a bounded contender stderr tail with native promises", async () => }, 10_000) test("evicts an unresponsive registered service before starting its replacement", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - const existing = Bun.spawn([process.execPath, fixture, registration, "hanging"], { - stdout: "ignore", - stderr: "inherit", - }) - processes.push(existing) - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + const existing = fixture.spawn("hanging") + await fixture.waitForFile() const original = await Bun.file(registration).json() const endpoint = await ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "delayed", "10"], + command: fixture.command("delayed", "10"), }) const replacement = await Bun.file(registration).json() + fixture.track(replacement.pid) expect((await Bun.file(registration + ".requests").text()).trim().split("\n")).toHaveLength(3) expect(await existing.exited).toBe(0) expect(replacement.pid).not.toBe(original.pid) expect(endpoint.url).toBe(replacement.url) - process.kill(replacement.pid, "SIGTERM") - await waitForExit(replacement.pid) }) test("signals the registered service process", async () => { - const registration = await setup("graceful") + await using fixture = await serviceFixture() + const registration = fixture.registration + fixture.spawn("graceful") + await fixture.waitForFile() await Service.stop({ file: registration }) expect(await Bun.file(registration + ".signal").text()).toBe("SIGTERM") expect(await Bun.file(registration).exists()).toBe(false) }) - -async function setup(mode: string) { - const directory = await temp() - const registration = join(directory, "service.json") - processes.push(Bun.spawn([process.execPath, fixture, registration, mode], { stdout: "ignore", stderr: "inherit" })) - await waitForFile(registration) - return registration -} - -async function temp() { - const directory = await mkdtemp(join(tmpdir(), "opencode-promise-service-")) - directories.push(directory) - return directory -} - -async function waitForFile(file: string) { - for (let attempt = 0; attempt < 600; attempt++) { - if (await Bun.file(file).exists()) return - await Bun.sleep(5) - } - throw new Error(`Timed out waiting for ${file}`) -} diff --git a/packages/client/test/service.test.ts b/packages/client/test/service.test.ts index 7f3da0e32c5..24768723aad 100644 --- a/packages/client/test/service.test.ts +++ b/packages/client/test/service.test.ts @@ -1,28 +1,18 @@ import { NodeFileSystem } from "@effect/platform-node" -import { afterEach, expect, test } from "bun:test" -import { Effect } from "effect" -import { mkdtemp, rm, writeFile } from "node:fs/promises" -import { tmpdir } from "node:os" -import { join } from "node:path" +import { expect, test } from "bun:test" +import { Effect, FileSystem } from "effect" +import { writeFile } from "node:fs/promises" import { Service, type EnsureReason } from "../src/effect/service" -import { accelerate, waitForExit } from "./fixture/service-timing" +import { serviceFixture } from "./fixture/service-fixture" +import { accelerate } from "./fixture/service-timing" -const fixture = join(import.meta.dir, "fixture/service.ts") const ensure = accelerate(Service.ensure) -const processes: Bun.Subprocess[] = [] -const directories: string[] = [] - -afterEach(async () => { - processes.forEach((process) => process.kill("SIGTERM")) - await Promise.all(processes.splice(0).map((process) => process.exited)) - await Promise.all(directories.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))) -}) test("a concurrent same-version start cannot invalidate a resolved endpoint", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - spawn(registration, "modern") - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + fixture.spawn("modern") + await fixture.waitForFile() const original = await Bun.file(registration).json() const starts: EnsureReason[] = [] @@ -34,7 +24,7 @@ test("a concurrent same-version start cannot invalidate a resolved endpoint", as onStart: (reason) => starts.push(reason), }), ) - await waitForFile(registration + ".first-request") + await fixture.waitForFile(registration + ".first-request") const resolved = await run(ensure({ file: registration, version: "test" })) expect(resolved.url).toBe(original.url) @@ -48,10 +38,10 @@ test("a concurrent same-version start cannot invalidate a resolved endpoint", as }) test("reuses a compatible registered service", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - const existing = spawn(registration, "compatible") - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + const existing = fixture.spawn("compatible") + await fixture.waitForFile() const starts: EnsureReason[] = [] const endpoint = await run( @@ -69,70 +59,65 @@ test("reuses a compatible registered service", async () => { }) test("adds configured environment variables when starting a service", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration const endpoint = await run( ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "environment"], + command: fixture.command("environment"), env: { OPENCODE_SERVICE_ENV_TEST: "configured" }, }), ) const info = await Bun.file(registration).json() + fixture.track(info.pid) - try { - expect(endpoint.url).toBe(info.url) - expect(await Bun.file(registration + ".environment").text()).toBe("configured") - } finally { - process.kill(info.pid, "SIGTERM") - await waitForExit(info.pid) - } + expect(endpoint.url).toBe(info.url) + expect(await Bun.file(registration + ".environment").text()).toBe("configured") }) test("replaces an incompatible registered service", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - const existing = spawn(registration, "incompatible") - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + const existing = fixture.spawn("incompatible") + await fixture.waitForFile() const starts: EnsureReason[] = [] const endpoint = await run( ensure({ file: registration, version: (version) => version.startsWith("2."), - command: [process.execPath, fixture, registration, "delayed-compatible", "10"], + command: fixture.command("delayed-compatible", "10"), onStart: (reason) => starts.push(reason), }), ) const replacement = await Bun.file(registration).json() + fixture.track(replacement.pid) expect(await existing.exited).toBe(0) expect(replacement.version).toBe("2.1.0-next.1") expect(endpoint.url).toBe(replacement.url) expect(starts).toEqual(["version-mismatch"]) - process.kill(replacement.pid, "SIGTERM") - await waitForExit(replacement.pid) }) test("waits for a registered service to finish starting", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - const process = spawn(registration, "starting") - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + const process = fixture.spawn("starting") + await fixture.waitForFile() const result = run(ensure({ file: registration, version: "test", command: [] })) - await waitForFile(registration + ".health-request") + await fixture.waitForFile(registration + ".health-request") expect(process.exitCode).toBe(null) await writeFile(registration + ".release", "") expect((await result).url).toBe((await Bun.file(registration).json()).url) }) test("reports a failed registered service without spawning", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - const process = spawn(registration, "failed-owner") - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + const process = fixture.spawn("failed-owner") + await fixture.waitForFile() await expect(run(ensure({ file: registration, version: "test", command: [] }))).rejects.toThrow( "Background service failed to start", @@ -141,35 +126,34 @@ test("reports a failed registered service without spawning", async () => { }) test("evicts an unresponsive registered service before starting its replacement", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - const existing = spawn(registration, "hanging") - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + const existing = fixture.spawn("hanging") + await fixture.waitForFile() const original = await Bun.file(registration).json() const endpoint = await run( ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "delayed", "10"], + command: fixture.command("delayed", "10"), }), ) const replacement = await Bun.file(registration).json() + fixture.track(replacement.pid) expect((await Bun.file(registration + ".requests").text()).trim().split("\n")).toHaveLength(3) expect(await existing.exited).toBe(0) expect(replacement.pid).not.toBe(original.pid) expect(endpoint.url).toBe(replacement.url) expect(await health(endpoint.url)).toEqual({ healthy: true, version: "test", pid: replacement.pid }) - process.kill(replacement.pid, "SIGTERM") - await waitForExit(replacement.pid) }) test("signals an unresponsive registered service process", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - const process = spawn(registration, "hanging") - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + const process = fixture.spawn("hanging") + await fixture.waitForFile() await run(Service.stop({ file: registration })) await process.exited @@ -178,30 +162,29 @@ test("signals an unresponsive registered service process", async () => { }) test("signals an incompatible service before starting its replacement", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - const existing = spawn(registration, "old") - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + const existing = fixture.spawn("old") + await fixture.waitForFile() const endpoint = await run( ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "delayed", "10"], + command: fixture.command("delayed", "10"), }), ) const replacement = await Bun.file(registration).json() + fixture.track(replacement.pid) expect(await existing.exited).toBe(0) expect(endpoint.url).toBe(replacement.url) - process.kill(replacement.pid, "SIGTERM") - await waitForExit(replacement.pid) }) test("a legacy health response is still replaced", async () => { - const directory = await temp() - const registration = join(directory, "service.json") - const existing = spawn(registration, "legacy") - await waitForFile(registration) + await using fixture = await serviceFixture() + const registration = fixture.registration + const existing = fixture.spawn("legacy") + await fixture.waitForFile() const starts: EnsureReason[] = [] const result = run(ensure({ file: registration, command: [], onStart: (reason) => starts.push(reason) })) @@ -212,67 +195,61 @@ test("a legacy health response is still replaced", async () => { }) test("waits for a slow winner while bounding lock probes", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration const endpoint = await run( ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "coordinated"], + command: fixture.command("coordinated"), }), ) const info = await Bun.file(registration).json() - try { - expect(endpoint.url).toBe(info.url) - expect(await health(endpoint.url)).toEqual({ healthy: true, version: "test", pid: info.pid }) - expect((await Bun.file(registration + ".starts").text()).trim().split("\n")).toHaveLength(2) - } finally { - process.kill(info.pid, "SIGTERM") - await waitForExit(info.pid) - } + fixture.track(info.pid) + + expect(endpoint.url).toBe(info.url) + expect(await health(endpoint.url)).toEqual({ healthy: true, version: "test", pid: info.pid }) + expect((await Bun.file(registration + ".starts").text()).trim().split("\n")).toHaveLength(2) }) test("waits for a live contender when another contender fails", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration const endpoint = await run( ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "coordinated-failed-loser", "300"], + command: fixture.command("coordinated-failed-loser", "300"), }), ) const info = await Bun.file(registration).json() - try { - expect(endpoint.url).toBe(info.url) - } finally { - process.kill(info.pid, "SIGTERM") - await waitForExit(info.pid) - } + fixture.track(info.pid) + + expect(endpoint.url).toBe(info.url) }) test("reports a contender that fails to start", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration await expect( run( ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "failed"], + command: fixture.command("failed"), }), ), ).rejects.toThrow("Server process exited with code 1") }) test("reports a bounded contender stderr tail", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration const error = await run( Service.ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "stderr-failed"], + command: fixture.command("stderr-failed"), }), ).catch((error: unknown) => error) @@ -283,85 +260,59 @@ test("reports a bounded contender stderr tail", async () => { }, 10_000) test("reports a contender terminated by a signal", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration await expect( run( ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "signal"], + command: fixture.command("signal"), }), ), ).rejects.toThrow(/Server process (terminated by|exited with code)/) }) test("reports a slow contender that eventually fails", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration await expect( run( ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "delayed-failed", "500"], + command: fixture.command("delayed-failed", "500"), }), ), ).rejects.toThrow("Server process exited with code 1") }) test("replaces an incompatible owner that appears during startup", async () => { - const directory = await temp() - const registration = join(directory, "service.json") + await using fixture = await serviceFixture() + const registration = fixture.registration const starting = run( ensure({ file: registration, version: "test", - command: [process.execPath, fixture, registration, "delayed", "500"], + command: fixture.command("delayed", "500"), }), ) - await waitForFile(registration + ".starts") - const old = spawn(registration, "old") - await waitForFile(registration) + await fixture.waitForFile(registration + ".starts") + const old = fixture.spawn("old") + await fixture.waitForFile() const endpoint = await starting const info = await Bun.file(registration).json() - try { - expect(endpoint.url).toBe(info.url) - expect(info.version).toBe("test") - await old.exited - } finally { - process.kill(info.pid, "SIGTERM") - await waitForExit(info.pid) - } + fixture.track(info.pid) + + expect(endpoint.url).toBe(info.url) + expect(info.version).toBe("test") + await old.exited }) -function run(effect: Effect.Effect) { +function run(effect: Effect.Effect) { return Effect.runPromise(effect.pipe(Effect.provide(NodeFileSystem.layer))) } -function spawn(registration: string, mode: string, ...args: string[]) { - const subprocess = Bun.spawn([process.execPath, fixture, registration, mode, ...args], { - stdout: "ignore", - stderr: "inherit", - }) - processes.push(subprocess) - return subprocess -} - -async function temp() { - const directory = await mkdtemp(join(tmpdir(), "opencode-client-service-")) - directories.push(directory) - return directory -} - -async function waitForFile(file: string) { - for (let attempt = 0; attempt < 600; attempt++) { - if (await Bun.file(file).exists()) return - await Bun.sleep(5) - } - throw new Error(`Timed out waiting for ${file}`) -} - async function health(url: string) { return fetch(new URL("/api/health", url), { signal: AbortSignal.timeout(1_000) }).then((response) => response.json()) }