From afff57db5bb7b79186491b251a66437be870bc39 Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Thu, 12 Mar 2026 02:21:20 -0700 Subject: [PATCH] test: remove conditional-expect anti-patterns from 3 test files (#2525) Replace `if (!r.ok) { expect(...) }` and `if (result.ok) { return }` guards with unconditional assertions using toThrow() or toMatchObject(). These conditional blocks silently skipped assertions when the condition evaluated the wrong way, providing false confidence. Also remove now-unused tryCatch imports from prompt-file-security.test.ts and security.test.ts. Co-authored-by: spawn-qa-bot Co-authored-by: Claude Sonnet 4.6 --- .../src/__tests__/prompt-file-security.test.ts | 17 ++++------------- packages/cli/src/__tests__/security.test.ts | 9 ++------- .../cli/src/__tests__/with-retry-result.test.ts | 12 ++++++------ 3 files changed, 12 insertions(+), 26 deletions(-) diff --git a/packages/cli/src/__tests__/prompt-file-security.test.ts b/packages/cli/src/__tests__/prompt-file-security.test.ts index 2c1afe4a..1588b3ea 100644 --- a/packages/cli/src/__tests__/prompt-file-security.test.ts +++ b/packages/cli/src/__tests__/prompt-file-security.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it } from "bun:test"; -import { tryCatch } from "@openrouter/spawn-shared"; import { validatePromptFilePath, validatePromptFileStats } from "../security.js"; describe("validatePromptFilePath", () => { @@ -82,12 +81,8 @@ describe("validatePromptFilePath", () => { }); it("should include helpful error message about exfiltration risk", () => { - const r = tryCatch(() => validatePromptFilePath("/home/user/.ssh/id_rsa")); - expect(r.ok).toBe(false); - if (!r.ok) { - expect(r.error.message).toContain("sent to the agent"); - expect(r.error.message).toContain("plain text file"); - } + expect(() => validatePromptFilePath("/home/user/.ssh/id_rsa")).toThrow("sent to the agent"); + expect(() => validatePromptFilePath("/home/user/.ssh/id_rsa")).toThrow("plain text file"); }); it("should reject SSH key files by filename pattern anywhere in path", () => { @@ -144,11 +139,7 @@ describe("validatePromptFileStats", () => { isFile: () => true, size: 5 * 1024 * 1024, }; - const r = tryCatch(() => validatePromptFileStats("large.bin", stats)); - expect(r.ok).toBe(false); - if (!r.ok) { - expect(r.error.message).toContain("5.0MB"); - expect(r.error.message).toContain("maximum is 1MB"); - } + expect(() => validatePromptFileStats("large.bin", stats)).toThrow("5.0MB"); + expect(() => validatePromptFileStats("large.bin", stats)).toThrow("maximum is 1MB"); }); }); diff --git a/packages/cli/src/__tests__/security.test.ts b/packages/cli/src/__tests__/security.test.ts index 56c85740..8e18b638 100644 --- a/packages/cli/src/__tests__/security.test.ts +++ b/packages/cli/src/__tests__/security.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it } from "bun:test"; -import { tryCatch } from "@openrouter/spawn-shared"; import { validateIdentifier, validatePrompt, validateScriptContent } from "../security.js"; /** @@ -431,12 +430,8 @@ describe("validatePrompt", () => { }); it("should provide helpful error message for command substitution", () => { - const r = tryCatch(() => validatePrompt("Run $(echo test)")); - expect(r.ok).toBe(false); - if (!r.ok) { - expect(r.error.message).toContain("shell syntax"); - expect(r.error.message).toContain("plain English"); - } + expect(() => validatePrompt("Run $(echo test)")).toThrow("shell syntax"); + expect(() => validatePrompt("Run $(echo test)")).toThrow("plain English"); }); it("should detect multiple dangerous patterns", () => { diff --git a/packages/cli/src/__tests__/with-retry-result.test.ts b/packages/cli/src/__tests__/with-retry-result.test.ts index 8153cc62..1aa98f99 100644 --- a/packages/cli/src/__tests__/with-retry-result.test.ts +++ b/packages/cli/src/__tests__/with-retry-result.test.ts @@ -149,12 +149,12 @@ describe("wrapSshCall", () => { it("wraps non-Error rejects into Error for Err", async () => { const result = await wrapSshCall(Promise.reject("string error")); - expect(result.ok).toBe(false); - if (result.ok) { - return; - } - expect(result.error).toBeInstanceOf(Error); - expect(result.error.message).toBe("string error"); + expect(result).toMatchObject({ + ok: false, + error: { + message: "string error", + }, + }); }); });