From ab192eb3f0ee93417c2bdcd36e2c478d9d794d5b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 8 May 2026 06:56:31 +0100 Subject: [PATCH] test: tighten helper assertion guards --- .../google/google-shared.test-helpers.ts | 6 ++-- src/agents/sandbox/fs-bridge.test-helpers.ts | 8 +++-- .../test-helpers/pi-tools-fs-helpers.ts | 30 +++++++++---------- src/gateway/device-authz.test-helpers.ts | 8 +++-- src/gateway/server.auth.control-ui.suite.ts | 30 +++++++++++++------ .../server.auth.default-token.suite.ts | 4 ++- src/gateway/server.auth.shared.ts | 22 ++++++++------ src/plugins/contracts/tts-contract-suites.ts | 15 +++++++--- .../resolve-target-error-cases.ts | 18 ++++++----- 9 files changed, 87 insertions(+), 54 deletions(-) diff --git a/extensions/google/google-shared.test-helpers.ts b/extensions/google/google-shared.test-helpers.ts index e710c742a4e..9023853c1a2 100644 --- a/extensions/google/google-shared.test-helpers.ts +++ b/extensions/google/google-shared.test-helpers.ts @@ -19,9 +19,9 @@ function makeZeroUsageSnapshot() { } export const asRecord = (value: unknown): Record => { - expect(value).toBeTruthy(); - expect(typeof value).toBe("object"); - expect(Array.isArray(value)).toBe(false); + if (!value || typeof value !== "object" || Array.isArray(value)) { + throw new Error("expected record"); + } return value as Record; }; diff --git a/src/agents/sandbox/fs-bridge.test-helpers.ts b/src/agents/sandbox/fs-bridge.test-helpers.ts index 154e819fa11..5bf552265f8 100644 --- a/src/agents/sandbox/fs-bridge.test-helpers.ts +++ b/src/agents/sandbox/fs-bridge.test-helpers.ts @@ -225,9 +225,11 @@ export async function expectMkdirpAllowsExistingDirectory(params?: { getDockerScript(args).includes("operation = sys.argv[1]") && getDockerArg(args, 1) === "mkdirp", ); - expect(mkdirCall).toBeDefined(); - const mountRoot = mkdirCall ? getDockerArg(mkdirCall[0], 2) : ""; - const relativePath = mkdirCall ? getDockerArg(mkdirCall[0], 3) : ""; + if (!mkdirCall) { + throw new Error("expected docker mkdirp call"); + } + const mountRoot = getDockerArg(mkdirCall[0], 2); + const relativePath = getDockerArg(mkdirCall[0], 3); expect(mountRoot).toBe("/workspace"); expect(relativePath).toBe("memory/kemik"); }); diff --git a/src/agents/test-helpers/pi-tools-fs-helpers.ts b/src/agents/test-helpers/pi-tools-fs-helpers.ts index 90fbf51576c..29a3ce7cd04 100644 --- a/src/agents/test-helpers/pi-tools-fs-helpers.ts +++ b/src/agents/test-helpers/pi-tools-fs-helpers.ts @@ -7,27 +7,27 @@ export function getTextContent(result?: { content?: TextResultBlock[] }) { return textBlock?.text ?? ""; } +function expectTool(tools: T[], name: string): T { + const tool = tools.find((entry) => entry.name === name); + if (!tool) { + throw new Error(`expected tool "${name}" in [${tools.map((entry) => entry.name).join(", ")}]`); + } + return tool; +} + export function expectReadWriteEditTools(tools: T[]) { - const readTool = tools.find((tool) => tool.name === "read"); - const writeTool = tools.find((tool) => tool.name === "write"); - const editTool = tools.find((tool) => tool.name === "edit"); - expect(readTool).toBeDefined(); - expect(writeTool).toBeDefined(); - expect(editTool).toBeDefined(); + expect(tools.map((tool) => tool.name)).toEqual(expect.arrayContaining(["read", "write", "edit"])); return { - readTool: readTool as T, - writeTool: writeTool as T, - editTool: editTool as T, + readTool: expectTool(tools, "read"), + writeTool: expectTool(tools, "write"), + editTool: expectTool(tools, "edit"), }; } export function expectReadWriteTools(tools: T[]) { - const readTool = tools.find((tool) => tool.name === "read"); - const writeTool = tools.find((tool) => tool.name === "write"); - expect(readTool).toBeDefined(); - expect(writeTool).toBeDefined(); + expect(tools.map((tool) => tool.name)).toEqual(expect.arrayContaining(["read", "write"])); return { - readTool: readTool as T, - writeTool: writeTool as T, + readTool: expectTool(tools, "read"), + writeTool: expectTool(tools, "write"), }; } diff --git a/src/gateway/device-authz.test-helpers.ts b/src/gateway/device-authz.test-helpers.ts index 45d2e93626b..cdad81c0344 100644 --- a/src/gateway/device-authz.test-helpers.ts +++ b/src/gateway/device-authz.test-helpers.ts @@ -86,7 +86,9 @@ export async function issueOperatorToken(params: { }); expect(rotated.ok).toBe(true); const token = rotated.ok ? rotated.entry.token : ""; - expect(token).toBeTruthy(); + if (!token) { + throw new Error(`expected rotated operator token for device ${paired.identity.deviceId}`); + } return { deviceId: paired.identity.deviceId, identityPath: paired.identityPath, @@ -96,7 +98,9 @@ export async function issueOperatorToken(params: { const device = await getPairedDevice(paired.identity.deviceId); const token = device?.tokens?.operator?.token ?? ""; - expect(token).toBeTruthy(); + if (!token) { + throw new Error(`expected operator token for paired device ${paired.identity.deviceId}`); + } expect(device?.approvedScopes).toEqual(params.approvedScopes); return { deviceId: paired.identity.deviceId, diff --git a/src/gateway/server.auth.control-ui.suite.ts b/src/gateway/server.auth.control-ui.suite.ts index 3f0aa6d0400..2420e978ce7 100644 --- a/src/gateway/server.auth.control-ui.suite.ts +++ b/src/gateway/server.auth.control-ui.suite.ts @@ -176,7 +176,6 @@ export function registerControlUiAndPairingSuite(): void { deviceId: string, ) => { const metadata = paired[deviceId]; - expect(metadata).toBeTruthy(); if (!metadata) { throw new Error(`Expected paired metadata for deviceId=${deviceId}`); } @@ -244,7 +243,9 @@ export function registerControlUiAndPairingSuite(): void { let device: Awaited>["device"] | null = null; if (tc.withUnpairedNodeDevice) { const challengeNonce = await readConnectChallengeNonce(ws); - expect(challengeNonce, tc.name).toBeTruthy(); + if (!challengeNonce) { + throw new Error(`expected connect challenge nonce for ${tc.name}`); + } ({ device } = await createSignedDevice({ token: null, role: "node", @@ -488,7 +489,9 @@ export function registerControlUiAndPairingSuite(): void { await withControlUiGatewayServer(async ({ port }) => { const staleDeviceWs = await openWs(port, { origin: originForPort(port) }); const challengeNonce = await readConnectChallengeNonce(staleDeviceWs); - expect(challengeNonce, "stale device challenge").toBeTruthy(); + if (!challengeNonce) { + throw new Error("expected stale device challenge nonce"); + } const { device } = await createSignedDevice({ token: "secret", scopes: [], @@ -735,7 +738,9 @@ export function registerControlUiAndPairingSuite(): void { (entry) => entry.deviceId === identity.deviceId, ); expect(pendingAfterRead).toHaveLength(0); - expect(await getPairedDevice(identity.deviceId)).toBeTruthy(); + if (!(await getPairedDevice(identity.deviceId))) { + throw new Error(`expected paired device ${identity.deviceId}`); + } wsRemoteRead.close(); const ws2 = await openWs(port, { host: "gateway.example" }); @@ -759,7 +764,9 @@ export function registerControlUiAndPairingSuite(): void { ); expect(pendingAfterAdmin).toHaveLength(1); expect(pendingAfterAdmin[0]?.scopes ?? []).toEqual(expect.arrayContaining(["operator.admin"])); - expect(await getPairedDevice(identity.deviceId)).toBeTruthy(); + if (!(await getPairedDevice(identity.deviceId))) { + throw new Error(`expected paired device ${identity.deviceId}`); + } ws2.close(); await server.close(); restoreGatewayToken(prevToken); @@ -941,8 +948,9 @@ export function registerControlUiAndPairingSuite(): void { const issuedOperatorToken = initialPayload?.auth?.deviceTokens?.find( (entry) => entry.role === "operator", )?.deviceToken; - expect(issuedDeviceToken).toBeDefined(); - expect(issuedOperatorToken).toBeDefined(); + if (!issuedDeviceToken || !issuedOperatorToken) { + throw new Error("expected issued device and operator tokens"); + } expect(initialPayload?.auth?.role).toBe("node"); expect(initialPayload?.auth?.scopes ?? []).toEqual([]); expect(initialPayload?.auth?.deviceTokens?.some((entry) => entry.role === "node")).toBe( @@ -1498,7 +1506,9 @@ export function registerControlUiAndPairingSuite(): void { const pendingUpgrade = (await listDevicePairing()).pending.find( (entry) => entry.deviceId === identity.deviceId, ); - expect(pendingUpgrade).toBeTruthy(); + if (!pendingUpgrade) { + throw new Error(`expected pending upgrade for device ${identity.deviceId}`); + } expect(pendingUpgrade?.scopes ?? []).toEqual(expect.arrayContaining(["operator.admin"])); const repaired = await getPairedDevice(identity.deviceId); expect(repaired?.role).toBe("operator"); @@ -1596,7 +1606,9 @@ export function registerControlUiAndPairingSuite(): void { expect(dockerCli.ok).toBe(true); const pending = await listDevicePairing(); expect(pending.pending.filter((entry) => entry.deviceId === identity.deviceId)).toEqual([]); - expect(await getPairedDevice(identity.deviceId)).toBeTruthy(); + if (!(await getPairedDevice(identity.deviceId))) { + throw new Error(`expected paired device ${identity.deviceId}`); + } } finally { wsDockerCli.close(); await server.close(); diff --git a/src/gateway/server.auth.default-token.suite.ts b/src/gateway/server.auth.default-token.suite.ts index 94bfa15eac3..860c6aa68cc 100644 --- a/src/gateway/server.auth.default-token.suite.ts +++ b/src/gateway/server.auth.default-token.suite.ts @@ -361,7 +361,9 @@ export function registerDefaultAuthTokenSuite(): void { const presence = helloOk?.snapshot?.presence; expect(Array.isArray(presence)).toBe(true); const mine = presence?.find((entry) => entry.deviceId === identity.deviceId); - expect(mine).toBeTruthy(); + if (!mine) { + throw new Error(`expected presence entry for device ${identity.deviceId}`); + } const presenceScopes = Array.isArray(mine?.scopes) ? mine?.scopes : []; expect(presenceScopes).toEqual([]); expect(presenceScopes).not.toContain("operator.admin"); diff --git a/src/gateway/server.auth.shared.ts b/src/gateway/server.auth.shared.ts index e1341b2956c..ac460428a1c 100644 --- a/src/gateway/server.auth.shared.ts +++ b/src/gateway/server.auth.shared.ts @@ -204,20 +204,22 @@ function resolveGatewayTokenOrEnv(): string { typeof (testState.gatewayAuth as { token?: unknown } | undefined)?.token === "string" ? ((testState.gatewayAuth as { token?: string }).token ?? undefined) : process.env.OPENCLAW_GATEWAY_TOKEN; - expect(typeof token).toBe("string"); - return token ?? ""; + if (typeof token !== "string") { + throw new Error("expected gateway token in test state or OPENCLAW_GATEWAY_TOKEN"); + } + return token; } async function approvePendingPairingIfNeeded() { const { approveDevicePairing, listDevicePairing } = await import("../infra/device-pairing.js"); const list = await listDevicePairing(); const pending = list.pending.at(0); - expect(pending?.requestId).toBeDefined(); - if (pending?.requestId) { - await approveDevicePairing(pending.requestId, { - callerScopes: pending.scopes ?? ["operator.admin"], - }); + if (!pending?.requestId) { + throw new Error("expected pending pairing request"); } + await approveDevicePairing(pending.requestId, { + callerScopes: pending.scopes ?? ["operator.admin"], + }); } async function configureTrustedProxyControlUiAuth() { @@ -325,8 +327,10 @@ async function resolvePairedTokenForDeviceIdentityPath(deviceIdentityPath: strin const paired = await getPairedDevice(identity.deviceId); const deviceToken = paired?.tokens?.operator?.token; expect(paired?.deviceId).toBe(identity.deviceId); - expect(deviceToken).toBeDefined(); - return { identity: { deviceId: identity.deviceId }, deviceToken: deviceToken ?? "" }; + if (!deviceToken) { + throw new Error(`expected operator token for paired device ${identity.deviceId}`); + } + return { identity: { deviceId: identity.deviceId }, deviceToken }; } async function startRateLimitedTokenServerWithPairedDeviceToken() { diff --git a/src/plugins/contracts/tts-contract-suites.ts b/src/plugins/contracts/tts-contract-suites.ts index bc942c39f78..76cdfdee056 100644 --- a/src/plugins/contracts/tts-contract-suites.ts +++ b/src/plugins/contracts/tts-contract-suites.ts @@ -944,7 +944,10 @@ export function describeTtsSummarizationContract() { `Invalid targetLength: ${testCase.targetLength}`, ); } else { - await expect(call, String(testCase.targetLength)).resolves.toBeDefined(); + await expect(call, String(testCase.targetLength)).resolves.toMatchObject({ + summary: expect.any(String), + inputLength: 4, + }); } }); @@ -1159,8 +1162,10 @@ export function describeTtsProviderRuntimeContract() { if (result.success) { throw new Error("expected synthesis failure"); } - expect(result.error).toBeDefined(); - const errorMessage = result.error ?? ""; + const errorMessage = result.error; + if (typeof errorMessage !== "string") { + throw new Error("expected synthesis failure error message"); + } expect(errorMessage).toBe("TTS conversion failed: openai: provider failed"); expect(errorMessage).not.toContain("TTS conversion failed: TTS conversion failed:"); expect(errorMessage.match(/TTS conversion failed:/g)).toHaveLength(1); @@ -1259,7 +1264,9 @@ export function describeTtsAutoApplyContract() { if (params.expectSamePayload) { expect(result).toBe(params.payload); } else { - expect(result.mediaUrl).toBeDefined(); + if (typeof result.mediaUrl !== "string" || result.mediaUrl.length === 0) { + throw new Error("expected auto TTS to attach mediaUrl"); + } } }); } diff --git a/src/test-helpers/resolve-target-error-cases.ts b/src/test-helpers/resolve-target-error-cases.ts index 42ce4a9a6c4..e1dabd29f35 100644 --- a/src/test-helpers/resolve-target-error-cases.ts +++ b/src/test-helpers/resolve-target-error-cases.ts @@ -19,6 +19,12 @@ export function installCommonResolveTargetErrorCases(params: { implicitAllowFrom: string[]; }) { const { resolveTarget, implicitAllowFrom } = params; + const expectResolveTargetError = (result: ResolveTargetResult) => { + expect(result.ok).toBe(false); + if (result.error === undefined) { + throw new Error("expected resolveTarget to return an error"); + } + }; it("should error on normalization failure with allowlist (implicit mode)", () => { const result = resolveTarget({ @@ -27,8 +33,7 @@ export function installCommonResolveTargetErrorCases(params: { allowFrom: implicitAllowFrom, }); - expect(result.ok).toBe(false); - expect(result.error).toBeDefined(); + expectResolveTargetError(result); }); it("should error when no target provided with allowlist", () => { @@ -38,8 +43,7 @@ export function installCommonResolveTargetErrorCases(params: { allowFrom: implicitAllowFrom, }); - expect(result.ok).toBe(false); - expect(result.error).toBeDefined(); + expectResolveTargetError(result); }); it("should error when no target and no allowlist", () => { @@ -49,8 +53,7 @@ export function installCommonResolveTargetErrorCases(params: { allowFrom: [], }); - expect(result.ok).toBe(false); - expect(result.error).toBeDefined(); + expectResolveTargetError(result); }); it("should handle whitespace-only target", () => { @@ -60,7 +63,6 @@ export function installCommonResolveTargetErrorCases(params: { allowFrom: [], }); - expect(result.ok).toBe(false); - expect(result.error).toBeDefined(); + expectResolveTargetError(result); }); }