From 18170a4313188dfe2ecb94ca111dee207f7efe3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E6=A9=98?= Date: Thu, 4 Jun 2026 12:31:17 +0000 Subject: [PATCH] refactor: extract validateCount, replace CLI spawn with direct import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract validateCount() from cmdThreadExec (throw instead of process.exit) - 5 validation tests now import validateCount directly (no subprocess) - Only --help tests still spawn CLI (need Commander output) - Test time: 1.7s → 475ms Fixes #61 --- .../src/__tests__/thread-step-count.test.ts | 54 +++++++++---------- packages/cli/src/commands/thread.ts | 10 ++-- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/packages/cli/src/__tests__/thread-step-count.test.ts b/packages/cli/src/__tests__/thread-step-count.test.ts index 44fb978..1d643b7 100644 --- a/packages/cli/src/__tests__/thread-step-count.test.ts +++ b/packages/cli/src/__tests__/thread-step-count.test.ts @@ -2,10 +2,15 @@ import { execFileSync } from "node:child_process"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { describe, expect, test } from "vitest"; +import { validateCount } from "../commands/thread.js"; const CLI_PATH = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "dist", "cli.js"); -function runCli(args: string[]): { stdout: string; stderr: string; exitCode: number } { +function runCli(args: string[]): { + stdout: string; + stderr: string; + exitCode: number; +} { try { const stdout = execFileSync("node", [CLI_PATH, ...args], { encoding: "utf8", @@ -14,7 +19,11 @@ function runCli(args: string[]): { stdout: string; stderr: string; exitCode: num }); return { stdout, stderr: "", exitCode: 0 }; } catch (e: unknown) { - const err = e as NodeJS.ErrnoException & { stdout?: string; stderr?: string; status?: number }; + const err = e as NodeJS.ErrnoException & { + stdout?: string; + stderr?: string; + status?: number; + }; return { stdout: err.stdout ?? "", stderr: err.stderr ?? "", @@ -23,52 +32,39 @@ function runCli(args: string[]): { stdout: string; stderr: string; exitCode: num } } -describe("thread exec --count CLI parsing", () => { - test("--help shows -c/--count option", { timeout: 30_000 }, () => { +describe("thread exec --count CLI parsing", { timeout: 30_000 }, () => { + test("--help shows -c/--count option", () => { const result = runCli(["thread", "exec", "--help"]); const combined = result.stdout + result.stderr; expect(combined).toContain("--count"); expect(combined).toContain("-c"); }); - test("description says 'one or more steps'", { timeout: 30_000 }, () => { + test("description says 'one or more steps'", () => { const result = runCli(["thread", "exec", "--help"]); const combined = result.stdout + result.stderr; expect(combined).toContain("one or more steps"); }); }); -describe("cmdThreadExec count logic", { timeout: 30_000 }, () => { - test("count=0 fails with validation error", () => { - const result = runCli(["thread", "exec", "FAKE_THREAD_ID", "-c", "0"]); - expect(result.exitCode).not.toBe(0); - expect(result.stderr).toContain("positive integer"); +describe("validateCount", () => { + test("count=0 throws validation error", () => { + expect(() => validateCount(0)).toThrow("positive integer"); }); - test("negative count fails with validation error", () => { - const result = runCli(["thread", "exec", "FAKE_THREAD_ID", "-c", "-1"]); - expect(result.exitCode).not.toBe(0); - expect(result.stderr).toContain("positive integer"); + test("negative count throws validation error", () => { + expect(() => validateCount(-1)).toThrow("positive integer"); }); - test("non-integer count fails with validation error", () => { - const result = runCli(["thread", "exec", "FAKE_THREAD_ID", "-c", "1.5"]); - expect(result.exitCode).not.toBe(0); - expect(result.stderr).toContain("positive integer"); + test("non-integer count throws validation error", () => { + expect(() => validateCount(1.5)).toThrow("positive integer"); }); - test("count=1 is the default (no -c flag)", () => { - // Without -c, it should attempt to run 1 step (failing on missing thread, not on count validation) - const result = runCli(["thread", "exec", "FAKE_THREAD_ID"]); - expect(result.exitCode).not.toBe(0); - // Should NOT contain "positive integer" error — should fail on thread lookup instead - expect(result.stderr).not.toContain("positive integer"); + test("count=1 passes validation", () => { + expect(() => validateCount(1)).not.toThrow(); }); - test("count=3 passes validation (fails on thread lookup)", () => { - const result = runCli(["thread", "exec", "FAKE_THREAD_ID", "-c", "3"]); - expect(result.exitCode).not.toBe(0); - // Should NOT contain "positive integer" error — should fail on thread/storage lookup - expect(result.stderr).not.toContain("positive integer"); + test("count=3 passes validation", () => { + expect(() => validateCount(3)).not.toThrow(); }); }); diff --git a/packages/cli/src/commands/thread.ts b/packages/cli/src/commands/thread.ts index 3baa949..5f08f27 100644 --- a/packages/cli/src/commands/thread.ts +++ b/packages/cli/src/commands/thread.ts @@ -1128,6 +1128,12 @@ export async function cmdThreadResume( }); } +export function validateCount(count: number): void { + if (count < 1 || !Number.isInteger(count)) { + throw new Error(`--count must be a positive integer, got: ${count}`); + } +} + export async function cmdThreadExec( storageRoot: string, threadId: ThreadId, @@ -1136,9 +1142,7 @@ export async function cmdThreadExec( background: boolean, backgroundWorker: boolean, ): Promise { - if (count < 1 || !Number.isInteger(count)) { - fail(`--count must be a positive integer, got: ${count}`); - } + validateCount(count); // Check if thread is already running in background (unless we ARE the background worker) if (!backgroundWorker) {