Skip to content

Commit e8cf33d

Browse files
la14-1spawn-qa-botclaude
authored
test: remove duplicate and theatrical tests (#3057)
* test: remove duplicate in-memory cache tests and fix missing cache reset Two tests verifying in-memory cache returns the same instance without re-fetching were duplicated across manifest.test.ts and manifest-cache-lifecycle.test.ts. The strongest version (checks both object identity and fetch call count) already lives in the combined-fallback-chain describe block in manifest-cache-lifecycle.test.ts, so the two weaker duplicates are removed. Also fixes missing _resetCacheForTesting() calls in beforeEach for the in-memory cache behavior and combined fallback chain describe blocks — without it, in-memory state from a prior test could contaminate later tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: remove duplicate and theatrical tests Consolidate 5 near-identical manifest rejection tests into a single data-driven loop, and collapse 4 identical logging-function smoke tests into a data-driven loop. Both changes eliminate copy-paste repetition while preserving exact test coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: spawn-qa-bot <qa@openrouter.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0bca96a commit e8cf33d

3 files changed

Lines changed: 70 additions & 111 deletions

File tree

‎packages/cli/src/__tests__/manifest-cache-lifecycle.test.ts‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { TestEnvironment } from "./test-helpers";
44
import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test";
55
import { existsSync, mkdirSync, rmSync, utimesSync, writeFileSync } from "node:fs";
66
import { join } from "node:path";
7-
import { agentKeys, countImplemented, loadManifest } from "../manifest";
7+
import { _resetCacheForTesting, agentKeys, countImplemented, loadManifest } from "../manifest";
88
import { createMockManifest, setupTestEnvironment, teardownTestEnvironment } from "./test-helpers";
99

1010
/**
@@ -239,6 +239,7 @@ describe("Manifest Cache Lifecycle", () => {
239239

240240
beforeEach(() => {
241241
env = setupTestEnvironment();
242+
_resetCacheForTesting();
242243
});
243244

244245
afterEach(() => {
@@ -255,22 +256,14 @@ describe("Manifest Cache Lifecycle", () => {
255256
// fetch should have been called at least twice (once per forceRefresh)
256257
expect(fetchMock.mock.calls.length).toBeGreaterThanOrEqual(2);
257258
});
258-
259-
it("should return same instance without forceRefresh", async () => {
260-
global.fetch = mock(() => Promise.resolve(new Response(JSON.stringify(mockManifest))));
261-
262-
const manifest1 = await loadManifest(true);
263-
const manifest2 = await loadManifest(false);
264-
265-
expect(manifest1).toBe(manifest2);
266-
});
267259
});
268260

269261
describe("combined fallback chain: invalid fetch + stale cache", () => {
270262
let env: TestEnvironment;
271263

272264
beforeEach(() => {
273265
env = setupTestEnvironment();
266+
_resetCacheForTesting();
274267
});
275268

276269
afterEach(() => {

‎packages/cli/src/__tests__/manifest.test.ts‎

Lines changed: 39 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -171,15 +171,6 @@ describe("manifest", () => {
171171
expect(global.fetch).toHaveBeenCalled();
172172
});
173173

174-
it("returns in-memory cache on second call without fetching", async () => {
175-
const fetchMock = mock(async () => new Response(JSON.stringify(mockManifest)));
176-
global.fetch = fetchMock;
177-
await loadManifest();
178-
const fetchCount = fetchMock.mock.calls.length;
179-
await loadManifest();
180-
expect(fetchMock.mock.calls.length).toBe(fetchCount);
181-
});
182-
183174
it("falls back to stale cache when fetch fails", async () => {
184175
const cacheDir = join(env.testDir, "spawn");
185176
mkdirSync(cacheDir, {
@@ -217,52 +208,33 @@ describe("manifest", () => {
217208
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
218209
});
219210

220-
it("throws when manifest from GitHub is invalid", async () => {
221-
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
222-
global.fetch = mock(
223-
async () =>
211+
const invalidManifestCases: Array<{
212+
label: string;
213+
fetchImpl: () => Promise<Response>;
214+
}> = [
215+
{
216+
label: "non-manifest shape",
217+
fetchImpl: async () =>
224218
new Response(
225219
JSON.stringify({
226220
not: "a manifest",
227221
}),
228222
),
229-
);
230-
231-
const cacheFile = join(env.testDir, "spawn", "manifest.json");
232-
if (existsSync(cacheFile)) {
233-
rmSync(cacheFile);
234-
}
235-
236-
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
237-
consoleSpy.mockRestore();
238-
});
239-
240-
it("rejects manifest with string agents field", async () => {
241-
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
242-
global.fetch = mock(
243-
async () =>
223+
},
224+
{
225+
label: "string agents field",
226+
fetchImpl: async () =>
244227
new Response(
245228
JSON.stringify({
246229
agents: "claude",
247230
clouds: {},
248231
matrix: {},
249232
}),
250233
),
251-
);
252-
253-
const cacheFile = join(env.testDir, "spawn", "manifest.json");
254-
if (existsSync(cacheFile)) {
255-
rmSync(cacheFile);
256-
}
257-
258-
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
259-
consoleSpy.mockRestore();
260-
});
261-
262-
it("rejects manifest with array clouds field", async () => {
263-
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
264-
global.fetch = mock(
265-
async () =>
234+
},
235+
{
236+
label: "array clouds field",
237+
fetchImpl: async () =>
266238
new Response(
267239
JSON.stringify({
268240
agents: {},
@@ -273,53 +245,38 @@ describe("manifest", () => {
273245
matrix: {},
274246
}),
275247
),
276-
);
277-
278-
const cacheFile = join(env.testDir, "spawn", "manifest.json");
279-
if (existsSync(cacheFile)) {
280-
rmSync(cacheFile);
281-
}
282-
283-
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
284-
consoleSpy.mockRestore();
285-
});
286-
287-
it("rejects manifest with numeric matrix field", async () => {
288-
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
289-
global.fetch = mock(
290-
async () =>
248+
},
249+
{
250+
label: "numeric matrix field",
251+
fetchImpl: async () =>
291252
new Response(
292253
JSON.stringify({
293254
agents: {},
294255
clouds: {},
295256
matrix: 42,
296257
}),
297258
),
298-
);
299-
300-
const cacheFile = join(env.testDir, "spawn", "manifest.json");
301-
if (existsSync(cacheFile)) {
302-
rmSync(cacheFile);
303-
}
304-
305-
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
306-
consoleSpy.mockRestore();
307-
});
308-
309-
it("throws when network errors occur and no cache exists", async () => {
310-
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
311-
global.fetch = mock(async () => {
312-
throw new Error("Network timeout");
259+
},
260+
{
261+
label: "network error",
262+
fetchImpl: async () => {
263+
throw new Error("Network timeout");
264+
},
265+
},
266+
];
267+
268+
for (const { label, fetchImpl } of invalidManifestCases) {
269+
it(`rejects invalid manifest (${label})`, async () => {
270+
const consoleSpy = spyOn(console, "error").mockImplementation(() => {});
271+
global.fetch = mock(fetchImpl);
272+
const cacheFile = join(env.testDir, "spawn", "manifest.json");
273+
if (existsSync(cacheFile)) {
274+
rmSync(cacheFile);
275+
}
276+
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
277+
consoleSpy.mockRestore();
313278
});
314-
315-
const cacheFile = join(env.testDir, "spawn", "manifest.json");
316-
if (existsSync(cacheFile)) {
317-
rmSync(cacheFile);
318-
}
319-
320-
await expect(loadManifest(true)).rejects.toThrow("Cannot load manifest");
321-
consoleSpy.mockRestore();
322-
});
279+
}
323280
});
324281
});
325282

‎packages/cli/src/__tests__/ui-cov.test.ts‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -62,25 +62,34 @@ afterEach(() => {
6262
// ── Logging functions ──────────────────────────────────────────────
6363

6464
describe("logging functions", () => {
65-
it("logInfo writes green text to stderr", () => {
66-
logInfo("test info");
67-
expect(stderrOutput.join("")).toContain("test info");
68-
});
69-
70-
it("logWarn writes yellow text to stderr", () => {
71-
logWarn("test warn");
72-
expect(stderrOutput.join("")).toContain("test warn");
73-
});
74-
75-
it("logError writes red text to stderr", () => {
76-
logError("test error");
77-
expect(stderrOutput.join("")).toContain("test error");
78-
});
79-
80-
it("logStep writes cyan text to stderr", () => {
81-
logStep("test step");
82-
expect(stderrOutput.join("")).toContain("test step");
83-
});
65+
for (const [fn, msg] of [
66+
[
67+
logInfo,
68+
"test info",
69+
],
70+
[
71+
logWarn,
72+
"test warn",
73+
],
74+
[
75+
logError,
76+
"test error",
77+
],
78+
[
79+
logStep,
80+
"test step",
81+
],
82+
] satisfies Array<
83+
[
84+
(msg: string) => void,
85+
string,
86+
]
87+
>) {
88+
it(`${fn.name} writes message to stderr`, () => {
89+
fn(msg);
90+
expect(stderrOutput.join("")).toContain(msg);
91+
});
92+
}
8493

8594
it("logStepInline writes message (newline-terminated in non-TTY)", () => {
8695
logStepInline("inline msg");

0 commit comments

Comments
 (0)