Skip to content

Commit f149073

Browse files
[Refactor] Add shared test utilities and pilot refactors (#1171)
* refactor: add shared test utilities and pilot refactors * refactor: reuse API test options in OpenAI specs * fix: cover shared test helpers and initial context state --------- Co-authored-by: Roomote <roomote@roomote.dev>
1 parent b2335fa commit f149073

19 files changed

Lines changed: 710 additions & 178 deletions

File tree

AGENTS.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,11 @@ Prefer the narrowest test layer that proves the behavior. This follows standard
4343
- Use `apps/vscode-e2e` only when the behavior depends on the real VS Code extension host, VS Code workspace APIs, extension activation, webview/extension messaging, file watcher behavior, or a complete user workflow.
4444
- Keep e2e tests focused on high-value smoke coverage across boundaries. Avoid placing detailed protocol, parsing, storage, retry, or edge-case assertions in e2e when they can be covered reliably at a lower layer.
4545
- When fixing a regression, add the regression test at the lowest layer that would have failed for the bug. Add an e2e test only if lower-level tests cannot represent the failure mode.
46+
47+
## Shared Test Utilities
48+
49+
- Use `src/test-utils/stream.ts` for mechanical async-stream setup and collection.
50+
- Use the typed helpers in `src/test-utils/api.ts`, `src/test-utils/fs.ts`, `src/test-utils/reset.ts`, and `src/test-utils/vscode.ts` when they remove repeated setup without hiding the scenario.
51+
- Keep provider-specific payloads, failure streams, and assertions inline when they explain the behavior under test.
52+
- Prefer shared helpers for mechanical duplication; use fixtures only when setup is reusable, typed, and independently disposable.
53+
- New helpers must preserve failure clarity, return fresh objects, and avoid `as any`; keep unavoidable VS Code structural casts inside the helper with a brief explanation.

src/api/providers/__tests__/openai-native.spec.ts

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ import { ApiProviderError, OpenAiServiceTier, SERVICE_TIER_KEY, serviceTiers } f
1818
import { OpenAiNativeHandler } from "../openai-native"
1919
import { ApiHandlerOptions } from "../../../shared/api"
2020
import { Package } from "../../../shared/package"
21+
import { expectRequestObjectContaining, makeApiHandlerOptions } from "../../../test-utils/api"
2122
import { asyncStreamFrom, collectStream } from "../../../test-utils/stream"
23+
import { deleteGlobalFetch } from "../../../test-utils/reset"
2224

2325
// Mock OpenAI client - now everything uses Responses API
2426
const mockResponsesCreate = vitest.fn()
@@ -41,18 +43,16 @@ const serviceTierPricingCases = [
4143
},
4244
]
4345

44-
vitest.mock("openai", () => {
45-
return {
46-
__esModule: true,
47-
default: vitest.fn().mockImplementation(function () {
48-
return {
49-
responses: {
50-
create: mockResponsesCreate,
51-
},
52-
}
53-
}),
54-
}
55-
})
46+
vitest.mock("openai", () => ({
47+
__esModule: true,
48+
default: vitest.fn().mockImplementation(function () {
49+
return {
50+
responses: {
51+
create: mockResponsesCreate,
52+
},
53+
}
54+
}),
55+
}))
5656

5757
describe("OpenAiNativeHandler", () => {
5858
let handler: OpenAiNativeHandler
@@ -66,24 +66,15 @@ describe("OpenAiNativeHandler", () => {
6666
]
6767

6868
beforeEach(() => {
69-
mockOptions = {
70-
apiModelId: "gpt-4.1",
71-
openAiNativeApiKey: "test-api-key",
72-
}
69+
mockOptions = makeApiHandlerOptions()
7370
handler = new OpenAiNativeHandler(mockOptions)
7471
mockResponsesCreate.mockClear()
7572
mockCaptureException.mockClear()
76-
// Clear fetch mock if it exists
77-
if ((global as any).fetch) {
78-
delete (global as any).fetch
79-
}
73+
deleteGlobalFetch()
8074
})
8175

8276
afterEach(() => {
83-
// Clean up fetch mock
84-
if ((global as any).fetch) {
85-
delete (global as any).fetch
86-
}
77+
deleteGlobalFetch()
8778
})
8879

8980
describe("constructor", () => {
@@ -152,7 +143,7 @@ describe("OpenAiNativeHandler", () => {
152143
await collectStream(handler.createMessage(systemPrompt, messages))
153144

154145
expect(mockResponsesCreate).toHaveBeenCalledWith(
155-
expect.objectContaining({ [SERVICE_TIER_KEY]: serviceTier }),
146+
expectRequestObjectContaining({ [SERVICE_TIER_KEY]: serviceTier }),
156147
expect.any(Object),
157148
)
158149
})

src/api/providers/__tests__/openai-usage-tracking.spec.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Anthropic } from "@anthropic-ai/sdk"
44

55
import { ApiHandlerOptions } from "../../../shared/api"
66
import { OpenAiHandler } from "../openai"
7+
import { makeApiHandlerOptions } from "../../../test-utils/api"
78
import { asyncStreamFrom, collectStream } from "../../../test-utils/stream"
89

910
const mockCreate = vitest.fn()
@@ -89,11 +90,11 @@ describe("OpenAiHandler with usage tracking fix", () => {
8990
let mockOptions: ApiHandlerOptions
9091

9192
beforeEach(() => {
92-
mockOptions = {
93+
mockOptions = makeApiHandlerOptions({
9394
openAiApiKey: "test-api-key",
9495
openAiModelId: "gpt-4",
9596
openAiBaseUrl: "https://api.openai.com/v1",
96-
}
97+
})
9798
handler = new OpenAiHandler(mockOptions)
9899
mockCreate.mockClear()
99100
})

src/api/providers/__tests__/openai.spec.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Anthropic } from "@anthropic-ai/sdk"
66
import OpenAI from "openai"
77
import { openAiModelInfoSaneDefaults, DEEP_SEEK_DEFAULT_TEMPERATURE } from "@roo-code/types"
88
import { Package } from "../../../shared/package"
9+
import { makeApiHandlerOptions } from "../../../test-utils/api"
910
import { asyncStreamFrom, collectStream } from "../../../test-utils/stream"
1011
import axios from "axios"
1112

@@ -88,11 +89,11 @@ describe("OpenAiHandler", () => {
8889
let mockOptions: ApiHandlerOptions
8990

9091
beforeEach(() => {
91-
mockOptions = {
92+
mockOptions = makeApiHandlerOptions({
9293
openAiApiKey: "test-api-key",
9394
openAiModelId: "gpt-4",
9495
openAiBaseUrl: "https://api.openai.com/v1",
95-
}
96+
})
9697
handler = new OpenAiHandler(mockOptions)
9798
mockCreate.mockClear()
9899
})

src/eslint-suppressions.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@
221221
},
222222
"api/providers/__tests__/openai-native.spec.ts": {
223223
"@typescript-eslint/no-explicit-any": {
224-
"count": 78
224+
"count": 74
225225
}
226226
},
227227
"api/providers/__tests__/openai-timeout.spec.ts": {
@@ -1196,7 +1196,7 @@
11961196
},
11971197
"integrations/editor/__tests__/DiffViewProvider.spec.ts": {
11981198
"@typescript-eslint/no-explicit-any": {
1199-
"count": 311
1199+
"count": 310
12001200
}
12011201
},
12021202
"integrations/editor/__tests__/EditorUtils.spec.ts": {

src/integrations/editor/__tests__/DiffViewProvider.spec.ts

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import * as vscode from "vscode"
33
import * as path from "path"
44
import delay from "delay"
55

6+
import { makeRange, makeTextDocument, makeTextEditor, makeUri } from "../../../test-utils/vscode"
7+
68
// Mock delay
79
vi.mock("delay", () => ({
810
default: vi.fn().mockResolvedValue(undefined),
@@ -356,26 +358,19 @@ describe("DiffViewProvider", () => {
356358
describe("scrollToFirstDiff method", () => {
357359
const setupEditor = (currentContent: string) => {
358360
const revealRange = vi.fn()
359-
// Mirror how VS Code reports lineCount: a trailing newline yields a final
360-
// empty line, so the count is the number of "\n"-delimited segments.
361-
const lineCount = currentContent === "" ? 0 : currentContent.split("\n").length
362-
const lines = currentContent.split("\n")
363-
const document = {
364-
uri: { fsPath: `${mockCwd}/mock-file-target.txt`, scheme: "file" },
361+
const document = makeTextDocument({
362+
uri: makeUri(`${mockCwd}/mock-file-target.txt`),
365363
getText: vi.fn().mockReturnValue(currentContent),
366-
lineCount,
367-
lineAt: vi.fn().mockImplementation((line: number) => ({ text: lines[line] ?? "" })),
368-
}
369-
const editor = {
364+
})
365+
const editor = makeTextEditor({
370366
document,
371-
selection: { active: { line: 0, character: 0 }, anchor: { line: 0, character: 0 } },
372-
visibleRanges: [{ start: { line: 0 }, end: { line: 0 } }],
367+
visibleRanges: [makeRange()],
373368
revealRange,
374-
}
369+
})
375370
;(diffViewProvider as any).activeDiffEditor = editor
376371
// Register the editor as the live modified-side editor so resolveLiveEditor
377372
// finds it by document identity, mirroring the runtime path.
378-
vi.mocked(vscode.window).visibleTextEditors = [editor as any]
373+
vi.mocked(vscode.window).visibleTextEditors = [editor]
379374
return revealRange
380375
}
381376

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { describe, expect, it, vi } from "vitest"
2+
3+
import { expectRequestObjectContaining, makeApiHandlerOptions, mockOpenAiResponsesClient } from "../api"
4+
5+
describe("API test utilities", () => {
6+
it("provides stable handler defaults with override support", () => {
7+
expect(makeApiHandlerOptions({ apiModelId: "gpt-5.6-sol" })).toMatchObject({
8+
apiModelId: "gpt-5.6-sol",
9+
openAiNativeApiKey: "test-api-key",
10+
})
11+
})
12+
13+
it("creates an OpenAI Responses API client mock", () => {
14+
const create = vi.fn()
15+
const client = mockOpenAiResponsesClient(create).default()
16+
17+
expect(client.responses.create).toBe(create)
18+
})
19+
20+
it("matches only the requested request fields", () => {
21+
expect({ model: "gpt-4.1", stream: true }).toEqual(expectRequestObjectContaining({ model: "gpt-4.1" }))
22+
})
23+
})
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { describe, expect, it, vi } from "vitest"
2+
3+
import { mockFsPromises, resetFsPromises } from "../fs"
4+
5+
describe("filesystem test utilities", () => {
6+
it("provides defaults and restores them after a test-specific override", async () => {
7+
const mock = mockFsPromises({ readFile: vi.fn().mockResolvedValue("custom content") })
8+
9+
expect(await mock.readFile()).toBe("custom content")
10+
11+
resetFsPromises(mock)
12+
13+
expect(await mock.readFile()).toBe("")
14+
expect(await mock.writeFile()).toBeUndefined()
15+
expect(await mock.access()).toBeUndefined()
16+
})
17+
})
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import nock from "nock"
2+
import { describe, expect, it, vi } from "vitest"
3+
4+
import { clearAllMocks, deleteGlobalFetch, resetNock, restoreGlobals } from "../reset"
5+
6+
describe("test reset utilities", () => {
7+
it("clears and restores Vitest mocks", () => {
8+
const mock = vi.fn()
9+
mock()
10+
11+
clearAllMocks()
12+
expect(mock).not.toHaveBeenCalled()
13+
14+
const target = { method: () => "original" }
15+
vi.spyOn(target, "method").mockReturnValue("mocked")
16+
expect(target.method()).toBe("mocked")
17+
18+
restoreGlobals()
19+
expect(target.method()).toBe("original")
20+
})
21+
22+
it("deletes the global fetch override", () => {
23+
const originalFetch = globalThis.fetch
24+
Object.defineProperty(globalThis, "fetch", {
25+
configurable: true,
26+
writable: true,
27+
value: vi.fn(),
28+
})
29+
30+
deleteGlobalFetch()
31+
32+
expect("fetch" in globalThis).toBe(false)
33+
34+
if (originalFetch) {
35+
Object.defineProperty(globalThis, "fetch", {
36+
configurable: true,
37+
writable: true,
38+
value: originalFetch,
39+
})
40+
}
41+
})
42+
43+
it("cleans pending nock scopes", () => {
44+
nock("https://test.example").get("/health").reply(200)
45+
expect(nock.pendingMocks()).toHaveLength(1)
46+
47+
resetNock()
48+
49+
expect(nock.pendingMocks()).toEqual([])
50+
})
51+
})
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, expect, it, vi } from "vitest"
2+
3+
import {
4+
makeDisposable,
5+
makeEventEmitter,
6+
makeExtensionContext,
7+
makePosition,
8+
makeRange,
9+
makeSelection,
10+
makeTextDocument,
11+
makeTextEditor,
12+
makeUri,
13+
makeWorkspaceConfiguration,
14+
} from "../vscode"
15+
16+
describe("VS Code test utilities", () => {
17+
it("creates the common VS Code value shapes", async () => {
18+
expect(makePosition(2, 3)).toEqual({ line: 2, character: 3 })
19+
expect(makeRange(1, 2, 3, 4)).toEqual({
20+
start: { line: 1, character: 2 },
21+
end: { line: 3, character: 4 },
22+
})
23+
expect(makeSelection(4, 5)).toEqual({
24+
anchor: { line: 4, character: 5 },
25+
active: { line: 4, character: 5 },
26+
})
27+
28+
const uri = makeUri("/tmp/test.ts", { scheme: "untitled" })
29+
const document = makeTextDocument({
30+
uri,
31+
getText: vi.fn().mockReturnValue("first\nsecond"),
32+
})
33+
const editor = makeTextEditor({ document })
34+
35+
expect(uri).toMatchObject({ fsPath: "/tmp/test.ts", scheme: "untitled" })
36+
expect(uri.toString()).toBe("/tmp/test.ts")
37+
expect(uri.toJSON()).toEqual({ fsPath: "/tmp/test.ts" })
38+
expect(document.lineCount).toBe(2)
39+
expect(document.lineAt(1).text).toBe("second")
40+
expect(document.getText()).toBe("first\nsecond")
41+
expect(document.getWordRangeAtPosition(makePosition())).toBeUndefined()
42+
expect(document.offsetAt(makePosition())).toBeUndefined()
43+
expect(document.positionAt(0)).toBeUndefined()
44+
expect(document.validateRange(makeRange())).toEqual(makeRange())
45+
expect(document.validatePosition(makePosition())).toEqual(makePosition())
46+
expect(makeTextDocument().getText()).toBe("")
47+
expect(editor.document).toBe(document)
48+
expect(await editor.edit(() => undefined)).toBe(true)
49+
50+
const disposable = makeDisposable()
51+
disposable.dispose()
52+
expect(disposable.dispose).toHaveBeenCalledOnce()
53+
})
54+
55+
it("supports event subscriptions and cleanup", () => {
56+
const emitter = makeEventEmitter<number>()
57+
const listener = vi.fn()
58+
const subscription = emitter.event(listener)
59+
60+
emitter.fire(1)
61+
expect(listener).toHaveBeenCalledWith(1)
62+
63+
subscription.dispose()
64+
emitter.fire(2)
65+
expect(listener).toHaveBeenCalledOnce()
66+
67+
emitter.dispose()
68+
})
69+
70+
it("creates configurable workspace settings", async () => {
71+
const configuration = makeWorkspaceConfiguration({ enabled: true })
72+
73+
expect(configuration.get("enabled")).toBe(true)
74+
expect(configuration.get("missing", "fallback")).toBe("fallback")
75+
expect(configuration.has("enabled")).toBe(true)
76+
expect(configuration.has("missing")).toBe(false)
77+
78+
await configuration.update("enabled", false)
79+
expect(configuration.update).toHaveBeenCalledWith("enabled", false)
80+
})
81+
82+
it("creates an extension context with fresh state containers", async () => {
83+
const context = makeExtensionContext({ extensionPath: "/custom/extension" })
84+
85+
expect(context.extensionPath).toBe("/custom/extension")
86+
expect(context.asAbsolutePath("dist")).toBe("/mock/extension/dist")
87+
expect(context.workspaceState.keys()).toEqual([])
88+
await context.workspaceState.update("key", "value")
89+
await context.secrets.store("key", "value")
90+
expect(context.workspaceState.update).toHaveBeenCalledWith("key", "value")
91+
expect(context.secrets.store).toHaveBeenCalledWith("key", "value")
92+
})
93+
})

0 commit comments

Comments
 (0)