Skip to content

Commit 9a08cc2

Browse files
sanchitrksanchitrk
authored
Merge pull request #40 from codeworksh/chore/prune-unit-tests
chore: prune low-signal unit tests in favour of E2E
2 parents 20bff56 + 3a461b7 commit 9a08cc2

116 files changed

Lines changed: 83 additions & 11106 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎AGENTS.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ This project uses private design documents under `.notes/` as implementation ref
6060
- Allow the design docs to be reviewed by subagents when see fit.
6161
- Propose improvements when see fit.
6262

63+
## Testing
64+
65+
- Never write unit tests after you write code.
66+
- Highly prefer E2E tests as the sole testing mechanism. Use them to verify complex features work (spawn the real CLI, boot the real runtime, hit the real provider or sandbox).
67+
- At the end of an E2E test, produce a verifiable and repeatable artifact under `__artifacts__/` (e.g. a path-normalised `toMatchFileSnapshot("./__artifacts__/<name>.json")` that fails on drift and regenerates with `-u`). Do not use `__snapshots__/`.
68+
- If you must test a system in isolation, first write down all the ways it could fail, then write the code. Only keep an isolated test if it catches a real bug the E2E tests miss (wire-protocol parsing, OAuth refresh, cancellation/concurrency, data integrity, security boundaries).
69+
6370
## Task Completion Requirements
6471

6572
- `vp check` and `vp run typecheck` must pass before considering tasks completed, unless otherwise specified.

‎CLAUDE.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ This project uses private design documents under `.notes/` as implementation ref
6060
- Allow the design docs to be reviewed by subagents when see fit.
6161
- Propose improvements when see fit.
6262

63+
## Testing
64+
65+
- Never write unit tests after you write code.
66+
- Highly prefer E2E tests as the sole testing mechanism. Use them to verify complex features work (spawn the real CLI, boot the real runtime, hit the real provider or sandbox).
67+
- At the end of an E2E test, produce a verifiable and repeatable artifact under `__artifacts__/` (e.g. a path-normalised `toMatchFileSnapshot("./__artifacts__/<name>.json")` that fails on drift and regenerates with `-u`). Do not use `__snapshots__/`.
68+
- If you must test a system in isolation, first write down all the ways it could fail, then write the code. Only keep an isolated test if it catches a real bug the E2E tests miss (wire-protocol parsing, OAuth refresh, cancellation/concurrency, data integrity, security boundaries).
69+
6370
## Task Completion Requirements
6471

6572
- `vp check` and `vp run typecheck` must pass before considering tasks completed, unless otherwise specified.

‎packages/aikit/test/abort.test.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,4 @@ describe("abort classification", () => {
2424
expect(message.stopReason).toBe("error");
2525
expect(message.failure).toMatchObject({ _tag: "Timeout", retryable: true });
2626
});
27-
28-
it("keeps a caller abort as aborted", async () => {
29-
const controller = new AbortController();
30-
setTimeout(() => controller.abort(), 20);
31-
const message = await run({ signal: controller.signal });
32-
expect(message.stopReason).toBe("aborted");
33-
expect(message.failure).toBeUndefined();
34-
});
3527
});

‎packages/aikit/test/catalog.test.ts‎

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -5,29 +5,6 @@ import { afterEach, beforeEach, describe, expect, it } from "vite-plus/test";
55
import * as ModelCatalog from "../src/model/catalog.ts";
66
import * as Model from "../src/model/model.ts";
77

8-
describe("ModelCatalog.path", () => {
9-
const configuredPath = process.env.CODEWORK_MODELS_FILE;
10-
11-
afterEach(() => {
12-
if (configuredPath === undefined) delete process.env.CODEWORK_MODELS_FILE;
13-
else process.env.CODEWORK_MODELS_FILE = configuredPath;
14-
});
15-
16-
it("resolves an explicit catalog path", () => {
17-
process.env.CODEWORK_MODELS_FILE = "fixtures/models.json";
18-
expect(ModelCatalog.path()).toBe(join(process.cwd(), "fixtures/models.json"));
19-
});
20-
21-
it("reads a configured path unless CODEWORK_MODELS_FILE is set", () => {
22-
delete process.env.CODEWORK_MODELS_FILE;
23-
Model.configureCatalog("configured/models.json");
24-
expect(Model.catalogPath()).toBe(join(process.cwd(), "configured/models.json"));
25-
26-
process.env.CODEWORK_MODELS_FILE = "fixtures/models.json";
27-
expect(Model.catalogPath()).toBe(join(process.cwd(), "fixtures/models.json"));
28-
});
29-
});
30-
318
describe("Model.reloadCatalog", () => {
329
const configuredPath = process.env.CODEWORK_MODELS_FILE;
3310
let directory: string;
@@ -70,37 +47,6 @@ describe("ModelCatalog.load", () => {
7047
await rm(directory, { recursive: true, force: true });
7148
});
7249

73-
it("loads a generated catalog object", async () => {
74-
const catalogPath = join(directory, "models.json");
75-
const catalog = { anthropic: { "claude-test": { id: "claude-test" } } };
76-
await writeFile(catalogPath, JSON.stringify(catalog));
77-
78-
await expect(ModelCatalog.load(catalogPath)).resolves.toEqual(catalog);
79-
});
80-
81-
it("classifies a missing catalog", async () => {
82-
const catalogPath = join(directory, "missing.json");
83-
84-
await expect(ModelCatalog.load(catalogPath)).rejects.toMatchObject({
85-
name: "ModelCatalogLoadError",
86-
data: {
87-
path: catalogPath,
88-
reason: "missing",
89-
message: `model catalog not found at ${catalogPath}`,
90-
},
91-
});
92-
});
93-
94-
it("classifies an empty catalog", async () => {
95-
const catalogPath = join(directory, "empty.json");
96-
await writeFile(catalogPath, " \n\t");
97-
98-
await expect(ModelCatalog.load(catalogPath)).rejects.toMatchObject({
99-
name: "ModelCatalogLoadError",
100-
data: { path: catalogPath, reason: "empty" },
101-
});
102-
});
103-
10450
it("classifies invalid JSON and non-object values", async () => {
10551
for (const [filename, content] of [
10652
["syntax.json", "{"],

‎packages/aikit/test/cli.oauth.test.ts‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,6 @@ describe("OAuthCommand", () => {
106106
expect(stderr()).toContain("Refreshed OpenAI Codex credentials");
107107
});
108108

109-
it("logs out and clears stored Codex credentials", async () => {
110-
await storage.set(makeCredentials());
111-
112-
await OAuthCommand.handler(args(path, { logout: true }));
113-
114-
await expect(storage.get()).resolves.toBeUndefined();
115-
expect(stderr()).toContain("Cleared OpenAI Codex credentials");
116-
});
117-
118109
it("prints Copilot status without exposing stored tokens", async () => {
119110
const copilotStorage = new JsonGitHubCopilotAuthStorage({ path });
120111
await copilotStorage.set({
@@ -132,21 +123,4 @@ describe("OAuthCommand", () => {
132123
expect(stdout()).toContain("Available models: 2");
133124
expect(stdout()).not.toContain("ghu_secret");
134125
});
135-
136-
it("reports missing Copilot credentials and fails the status check", async () => {
137-
await OAuthCommand.handler(args(path, { openaiCodex: false, githubCopilot: true, status: true }));
138-
139-
expect(process.exitCode).toBe(1);
140-
expect(stderr()).toContain("no GitHub Copilot credentials found");
141-
});
142-
143-
it("logs out and clears stored Copilot credentials", async () => {
144-
const copilotStorage = new JsonGitHubCopilotAuthStorage({ path });
145-
await copilotStorage.set({ access: "ghu_secret", refresh: "ghu_secret", expires: 0 });
146-
147-
await OAuthCommand.handler(args(path, { openaiCodex: false, githubCopilot: true, logout: true }));
148-
149-
await expect(copilotStorage.get()).resolves.toBeUndefined();
150-
expect(stderr()).toContain("Cleared GitHub Copilot credentials");
151-
});
152126
});

‎packages/aikit/test/cost.cachewrite1h.test.ts‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -18,30 +18,6 @@ describe("Model.calculateCost with 1h cache writes", () => {
1818
// 600k * 6.25/Mtok + 400k * 10/Mtok = 3.75 + 4.0 = 7.75
1919
expect(usage.cost.cacheWrite).toBeCloseTo(7.75, 10);
2020
});
21-
22-
it("falls back to the 5m rate when no breakdown is reported", () => {
23-
const usage = makeUsage({ cacheWrite: 1_000_000 });
24-
25-
Model.calculateCost(opus, usage);
26-
27-
expect(usage.cost.cacheWrite).toBeCloseTo(6.25, 10);
28-
});
29-
30-
it("prices an all-1h write entirely at 2x input", () => {
31-
const usage = makeUsage({ cacheWrite: 1_000_000, cacheWrite1h: 1_000_000 });
32-
33-
Model.calculateCost(opus, usage);
34-
35-
expect(usage.cost.cacheWrite).toBeCloseTo(10, 10);
36-
});
37-
38-
it("includes the 1h premium in the total", () => {
39-
const usage = makeUsage({ input: 1_000_000, cacheWrite: 1_000_000, cacheWrite1h: 400_000 });
40-
41-
Model.calculateCost(opus, usage);
42-
43-
expect(usage.cost.total).toBeCloseTo(5 + 7.75, 10);
44-
});
4521
});
4622

4723
describe("mapUsage 1h cache write breakdown", () => {
@@ -65,18 +41,4 @@ describe("mapUsage 1h cache write breakdown", () => {
6541
expect(result.cacheWrite1h).toBe(400_000);
6642
expect(result.cost.cacheWrite).toBeCloseTo(7.75, 10);
6743
});
68-
69-
it("omits the split when the provider does not report one", () => {
70-
const result = mapUsage(usage, opus, 1, { anthropic: { usage: { input_tokens: 100 } } });
71-
72-
expect(result.cacheWrite1h).toBeUndefined();
73-
expect(result.cost.cacheWrite).toBeCloseTo(6.25, 10);
74-
});
75-
76-
it("ignores metadata from unrelated providers", () => {
77-
const result = mapUsage(usage, opus, 1, { openai: { serviceTier: "flex" } });
78-
79-
expect(result.cacheWrite1h).toBeUndefined();
80-
expect(result.cost.cacheWrite).toBeCloseTo(6.25, 10);
81-
});
8244
});

‎packages/aikit/test/estimate.test.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,6 @@ function assistant(created: number, totalTokens: number): Message.AssistantMessa
2323
});
2424
}
2525

26-
describe("estimateTextTokens", () => {
27-
it("counts four characters to the token, rounding up", () => {
28-
expect(estimateTextTokens("")).toBe(0);
29-
expect(estimateTextTokens("abc")).toBe(1);
30-
expect(estimateTextTokens("x".repeat(4_000))).toBe(1_000);
31-
});
32-
});
33-
3426
describe("estimateMessageTokens", () => {
3527
it("charges a flat cost for images rather than their base64 length", () => {
3628
const withImage = Message.createUserMessage({

‎packages/aikit/test/eventstream.test.ts‎

Lines changed: 1 addition & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import { describe, expect, it } from "vite-plus/test";
2-
import type * as Event from "../src/event/event.ts";
3-
import { AssistantMessageEventStream, EventStream } from "../src/utils/eventstream.ts";
4-
import { makeAssistantMessage, makeModel } from "./utils/fixtures.ts";
2+
import { EventStream } from "../src/utils/eventstream.ts";
53

64
type TestEvent = { kind: "data" | "final"; value: number };
75

@@ -21,45 +19,6 @@ async function collect<T>(iterable: AsyncIterable<T>): Promise<T[]> {
2119
}
2220

2321
describe("EventStream", () => {
24-
it("delivers queued events in order when pushed before iteration", async () => {
25-
const stream = makeStream();
26-
stream.push({ kind: "data", value: 1 });
27-
stream.push({ kind: "data", value: 2 });
28-
stream.push({ kind: "final", value: 3 });
29-
30-
const events = await collect(stream);
31-
expect(events.map((e) => e.value)).toEqual([1, 2, 3]);
32-
});
33-
34-
it("delivers events to a consumer that is already waiting", async () => {
35-
const stream = makeStream();
36-
const consumer = collect(stream);
37-
38-
// Yield to let the consumer start awaiting before any push.
39-
await new Promise((resolve) => setTimeout(resolve, 0));
40-
stream.push({ kind: "data", value: 1 });
41-
stream.push({ kind: "final", value: 2 });
42-
43-
const events = await consumer;
44-
expect(events.map((e) => e.value)).toEqual([1, 2]);
45-
});
46-
47-
it("ends iteration after the completing event without an explicit end()", async () => {
48-
const stream = makeStream();
49-
stream.push({ kind: "final", value: 42 });
50-
51-
const events = await collect(stream);
52-
expect(events).toHaveLength(1);
53-
});
54-
55-
it("resolves result() from the completing event", async () => {
56-
const stream = makeStream();
57-
stream.push({ kind: "data", value: 1 });
58-
stream.push({ kind: "final", value: 42 });
59-
60-
await expect(stream.result()).resolves.toBe(42);
61-
});
62-
6322
it("ignores pushes after completion", async () => {
6423
const stream = makeStream();
6524
stream.push({ kind: "final", value: 1 });
@@ -70,23 +29,6 @@ describe("EventStream", () => {
7029
await expect(stream.result()).resolves.toBe(1);
7130
});
7231

73-
it("end() releases a waiting consumer", async () => {
74-
const stream = makeStream();
75-
const consumer = collect(stream);
76-
77-
await new Promise((resolve) => setTimeout(resolve, 0));
78-
stream.end();
79-
80-
const events = await consumer;
81-
expect(events).toEqual([]);
82-
});
83-
84-
it("end(result) resolves result() without a completing event", async () => {
85-
const stream = makeStream();
86-
stream.end(7);
87-
await expect(stream.result()).resolves.toBe(7);
88-
});
89-
9032
it("drains queued events even after end()", async () => {
9133
const stream = makeStream();
9234
stream.push({ kind: "data", value: 1 });
@@ -96,32 +38,3 @@ describe("EventStream", () => {
9638
expect(events.map((e) => e.value)).toEqual([1]);
9739
});
9840
});
99-
100-
describe("AssistantMessageEventStream", () => {
101-
const model = makeModel();
102-
103-
it("resolves result() with the message from a done event", async () => {
104-
const stream = new AssistantMessageEventStream();
105-
const message = makeAssistantMessage(model, { stopReason: "stop" });
106-
107-
stream.push({ type: "start", partial: message });
108-
stream.push({ type: "done", reason: "stop", message });
109-
stream.end();
110-
111-
const events = await collect(stream as AsyncIterable<Event.LLMMessageEvent>);
112-
expect(events.map((e) => e.type)).toEqual(["start", "done"]);
113-
await expect(stream.result()).resolves.toBe(message);
114-
});
115-
116-
it("resolves result() with the message from an error event", async () => {
117-
const stream = new AssistantMessageEventStream();
118-
const message = makeAssistantMessage(model, { stopReason: "error", errorMessage: "boom" });
119-
120-
stream.push({ type: "error", reason: "error", error: message });
121-
stream.end();
122-
123-
const result = await stream.result();
124-
expect(result.stopReason).toBe("error");
125-
expect(result.errorMessage).toBe("boom");
126-
});
127-
});

‎packages/aikit/test/jsonparse.test.ts‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,39 +2,6 @@ import { describe, expect, it } from "vite-plus/test";
22
import { parseStreamingJson } from "../src/utils/jsonparse.ts";
33

44
describe("parseStreamingJson", () => {
5-
describe("empty input", () => {
6-
it("returns an empty object for undefined", () => {
7-
expect(parseStreamingJson(undefined)).toEqual({});
8-
});
9-
10-
it("returns an empty object for an empty string", () => {
11-
expect(parseStreamingJson("")).toEqual({});
12-
});
13-
14-
it("returns an empty object for whitespace-only input", () => {
15-
expect(parseStreamingJson(" \n\t ")).toEqual({});
16-
});
17-
});
18-
19-
describe("complete JSON", () => {
20-
it("parses a complete object", () => {
21-
expect(parseStreamingJson('{"a": 1, "b": "two"}')).toEqual({ a: 1, b: "two" });
22-
});
23-
24-
it("parses nested structures", () => {
25-
expect(parseStreamingJson('{"items": [{"id": 1}, {"id": 2}], "done": true}')).toEqual({
26-
items: [{ id: 1 }, { id: 2 }],
27-
done: true,
28-
});
29-
});
30-
31-
it("parses valid escape sequences", () => {
32-
expect(parseStreamingJson('{"text": "line1\\nline2\\ttabbed"}')).toEqual({
33-
text: "line1\nline2\ttabbed",
34-
});
35-
});
36-
});
37-
385
describe("partial JSON during streaming", () => {
396
it("parses an object missing its closing brace", () => {
407
expect(parseStreamingJson('{"a": 1')).toEqual({ a: 1 });

0 commit comments

Comments
 (0)