Skip to content

Commit e8322b4

Browse files
mini2sunknownzjc
mini2s
and
unknownzjc
authored
Dev codereview (#558)
* fix(code-review): update logging and filter cached issues based on file existence (#557) * test: enhance vscode mocks across test files to improve test reliability --------- Co-authored-by: unknown_ <[email protected]>
1 parent efbe9f2 commit e8322b4

File tree

21 files changed

+674
-96
lines changed

21 files changed

+674
-96
lines changed

‎src/__mocks__/vscode.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export const window = {
9292
onDidCloseTerminal: () => mockDisposable,
9393
createTextEditorDecorationType: () => ({ dispose: () => {} }),
9494
showTextDocument: () => {},
95+
tabGroups: { all: [] },
9596
}
9697

9798
export const commands = {

‎src/api/providers/__tests__/openrouter.spec.ts‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,35 @@
11
// npx vitest run src/api/providers/__tests__/openrouter.spec.ts
22

33
// Mock vscode first to avoid import errors
4-
vitest.mock("vscode", () => ({}))
4+
vitest.mock("vscode", () => ({
5+
env: {
6+
uriScheme: "vscode",
7+
},
8+
RelativePattern: vitest.fn(),
9+
workspace: {
10+
getConfiguration: vitest.fn(),
11+
createFileSystemWatcher: vitest.fn(() => ({
12+
onDidChange: vitest.fn(),
13+
onDidCreate: vitest.fn(),
14+
onDidDelete: vitest.fn(),
15+
dispose: vitest.fn(),
16+
})),
17+
},
18+
window: {
19+
showInformationMessage: vitest.fn(),
20+
createTextEditorDecorationType: vitest.fn(() => ({
21+
dispose: vitest.fn(),
22+
})),
23+
createOutputChannel: vitest.fn(() => ({
24+
appendLine: vitest.fn(),
25+
append: vitest.fn(),
26+
clear: vitest.fn(),
27+
show: vitest.fn(),
28+
hide: vitest.fn(),
29+
dispose: vitest.fn(),
30+
})),
31+
},
32+
}))
533

634
import { Anthropic } from "@anthropic-ai/sdk"
735
import OpenAI from "openai"
@@ -13,6 +41,16 @@ import { Package } from "../../../shared/package"
1341
// Mock dependencies
1442
vitest.mock("openai")
1543
vitest.mock("delay", () => ({ default: vitest.fn(() => Promise.resolve()) }))
44+
45+
vitest.mock("os", () => ({
46+
tmpdir: vitest.fn(() => "/tmp"),
47+
homedir: vitest.fn(() => "/home/user"),
48+
}))
49+
50+
vitest.mock("path", () => ({
51+
join: vitest.fn((...paths) => paths.join("/")),
52+
sep: "/",
53+
}))
1654
vitest.mock("../fetchers/modelCache", () => ({
1755
getModels: vitest.fn().mockImplementation(() => {
1856
return Promise.resolve({

‎src/core/config/__tests__/ContextProxy.spec.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,36 @@ vi.mock("vscode", () => ({
1515
Production: 2,
1616
Test: 3,
1717
},
18+
env: {
19+
openExternal: vi.fn().mockResolvedValue(true),
20+
uriScheme: "vscode",
21+
},
22+
RelativePattern: class {
23+
constructor(base: any, pattern: any) {
24+
this.base = base
25+
this.pattern = pattern
26+
}
27+
base: any
28+
pattern: any
29+
},
30+
workspace: {
31+
createFileSystemWatcher: vi.fn(() => ({
32+
onDidCreate: vi.fn(() => ({ dispose: vi.fn() })),
33+
onDidChange: vi.fn(() => ({ dispose: vi.fn() })),
34+
onDidDelete: vi.fn(() => ({ dispose: vi.fn() })),
35+
dispose: vi.fn(),
36+
})),
37+
},
38+
window: {
39+
createTextEditorDecorationType: vi.fn(() => ({ dispose: vi.fn() })),
40+
createOutputChannel: vi.fn(() => ({
41+
appendLine: vi.fn(),
42+
append: vi.fn(),
43+
clear: vi.fn(),
44+
show: vi.fn(),
45+
dispose: vi.fn(),
46+
})),
47+
},
1848
}))
1949

2050
describe("ContextProxy", () => {

‎src/core/config/__tests__/importExport.spec.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,38 @@ vi.mock("vscode", () => ({
2222
showSaveDialog: vi.fn(),
2323
showErrorMessage: vi.fn(),
2424
showInformationMessage: vi.fn(),
25+
createTextEditorDecorationType: vi.fn(() => ({ dispose: vi.fn() })),
26+
createOutputChannel: vi.fn(() => ({
27+
appendLine: vi.fn(),
28+
append: vi.fn(),
29+
clear: vi.fn(),
30+
show: vi.fn(),
31+
dispose: vi.fn(),
32+
})),
2533
},
2634
Uri: {
2735
file: vi.fn((filePath) => ({ fsPath: filePath })),
2836
},
37+
env: {
38+
openExternal: vi.fn().mockResolvedValue(true),
39+
uriScheme: "vscode",
40+
},
41+
RelativePattern: class {
42+
constructor(base: any, pattern: any) {
43+
this.base = base
44+
this.pattern = pattern
45+
}
46+
base: any
47+
pattern: any
48+
},
49+
workspace: {
50+
createFileSystemWatcher: vi.fn(() => ({
51+
onDidCreate: vi.fn(() => ({ dispose: vi.fn() })),
52+
onDidChange: vi.fn(() => ({ dispose: vi.fn() })),
53+
onDidDelete: vi.fn(() => ({ dispose: vi.fn() })),
54+
dispose: vi.fn(),
55+
})),
56+
},
2957
}))
3058

3159
vi.mock("fs/promises", () => ({
@@ -54,6 +82,7 @@ vi.mock("os", () => ({
5482
homedir: vi.fn(() => "/mock/home"),
5583
},
5684
homedir: vi.fn(() => "/mock/home"),
85+
tmpdir: vi.fn(() => "/tmp"),
5786
}))
5887

5988
vi.mock("../../../utils/safeWriteJson")

‎src/core/costrict/code-review/__tests__/codeReviewService.spec.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ import {
2525
} from "../../../../shared/codeReview"
2626

2727
// Mock vscode
28-
vi.mock("vscode", () => ({
28+
vi.mock("vscode", async (importOriginal) => ({
29+
...(await importOriginal()),
2930
Uri: {
3031
joinPath: vi.fn().mockReturnValue({
3132
scheme: "file",
@@ -68,6 +69,16 @@ vi.mock("vscode", () => ({
6869
appendLine: vi.fn(),
6970
dispose: vi.fn(),
7071
}),
72+
createTextEditorDecorationType: vi.fn().mockReturnValue({
73+
setDecorationOptions: vi.fn(),
74+
setGutterIconPath: vi.fn(),
75+
setHoverMessage: vi.fn(),
76+
setColor: vi.fn(),
77+
setOverviewRulerColor: vi.fn(),
78+
setInvisible: vi.fn(),
79+
setZIndex: vi.fn(),
80+
dispose: vi.fn(),
81+
}),
7182
},
7283
workspace: {
7384
createFileSystemWatcher: vi.fn().mockReturnValue({

‎src/core/costrict/code-review/codeReviewService.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ import { TelemetryService } from "@roo-code/telemetry"
4949
import { CodeReviewErrorType, type TelemetryErrorType } from "../telemetry"
5050
import { COSTRICT_DEFAULT_HEADERS } from "../../../shared/headers"
5151
import { zgsmCodebaseIndexManager } from "../codebase-index"
52-
import { toRelativePath } from "../../../utils/path"
52+
import { fileExistsAtPath } from "../../../utils/fs"
5353
/**
5454
* Code Review Service - Singleton
5555
*
@@ -226,7 +226,7 @@ export class CodeReviewService {
226226
task.on(RooCodeEventName.TaskAskResponded, () => {
227227
if (!isCompleted) {
228228
const messageCount = task.clineMessages.length
229-
229+
console.log("TaskAskResponded", task.clineMessages)
230230
let progress = 0
231231
if (messageCount <= 10) {
232232
progress = messageCount * 0.05
@@ -243,7 +243,7 @@ export class CodeReviewService {
243243
try {
244244
this.logger.info("[CodeReview] Review Task completed")
245245
isCompleted = true
246-
// console.log("task messages", task.clineMessages)
246+
console.log("TaskCompleted", task.clineMessages)
247247
const message = task.clineMessages.find(
248248
(msg) => msg.type === "say" && msg.text?.includes("I-AM-CODE-REVIEW-REPORT-V1"),
249249
)
@@ -258,7 +258,9 @@ export class CodeReviewService {
258258
review_progress: "",
259259
total: issues?.length ?? 0,
260260
}
261-
this.updateCachedIssues(issues)
261+
this.updateCachedIssues(
262+
issues.filter((issue) => fileExistsAtPath(path.resolve(provider.cwd, issue.file_path))),
263+
)
262264
}
263265
}
264266
} finally {
@@ -345,6 +347,7 @@ export class CodeReviewService {
345347
review_progress: "",
346348
total: 0,
347349
}
350+
this.commentService?.clearAllCommentThreads()
348351
this.sendReviewTaskUpdateMessage(TaskStatus.INITIAL, {
349352
issues: [],
350353
progress: 0,

‎src/core/costrict/codebase-index/workspace-event-monitor.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ vi.mock("vscode", async (importOriginal) => {
3737
},
3838
window: {
3939
activeTextEditor: null,
40+
createTextEditorDecorationType: vi.fn(),
41+
createOutputChannel: vi.fn(),
4042
},
4143
Uri: {
4244
file: (path: string) => ({ fsPath: path }),

‎src/core/environment/__tests__/getEnvironmentDetails.spec.ts‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,6 @@ import { RooIgnoreController } from "../../ignore/RooIgnoreController"
1919
import { formatResponse } from "../../prompts/responses"
2020
import { Task } from "../../task/Task"
2121

22-
vi.mock("vscode", () => ({
23-
window: {
24-
tabGroups: { all: [], onDidChangeTabs: vi.fn() },
25-
visibleTextEditors: [],
26-
},
27-
env: {
28-
language: "en-US",
29-
},
30-
}))
31-
3222
vi.mock("p-wait-for", () => ({
3323
default: vi.fn(),
3424
}))
@@ -93,7 +83,7 @@ describe("getEnvironmentDetails", () => {
9383
getAndClearRecentlyModifiedFiles: vi.fn().mockReturnValue([]),
9484
} as unknown as FileContextTracker,
9585
rooIgnoreController: {
96-
filterPaths: vi.fn((paths: string[]) => paths.join("\n")),
86+
filterPaths: vi.fn((paths: string[] = []) => paths.join("\n")),
9787
cwd: mockCwd,
9888
ignoreInstance: {},
9989
disposables: [],

‎src/core/prompts/__tests__/add-custom-instructions.spec.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,30 @@ vi.mock("vscode", () => ({
126126
workspace: {
127127
workspaceFolders: [{ uri: { fsPath: "/test/path" } }],
128128
getWorkspaceFolder: vi.fn().mockReturnValue({ uri: { fsPath: "/test/path" } }),
129+
createFileSystemWatcher: () => ({
130+
onDidCreate: () => ({ dispose: () => {} }),
131+
onDidChange: () => ({ dispose: () => {} }),
132+
onDidDelete: () => ({ dispose: () => {} }),
133+
dispose: () => {},
134+
}),
129135
},
130136
window: {
131137
activeTextEditor: undefined,
138+
createTextEditorDecorationType: () => ({ dispose: () => {} }),
139+
createOutputChannel: () => ({
140+
appendLine: () => {},
141+
show: () => {},
142+
dispose: () => {},
143+
}),
144+
tabGroups: { all: [] },
145+
},
146+
RelativePattern: class {
147+
base: any
148+
pattern: any
149+
constructor(base: any, pattern: any) {
150+
this.base = base
151+
this.pattern = pattern
152+
}
132153
},
133154
EventEmitter: vi.fn().mockImplementation(() => ({
134155
event: vi.fn(),

‎src/core/prompts/__tests__/custom-system-prompt.spec.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,30 @@ vi.mock("vscode", () => ({
66
workspace: {
77
workspaceFolders: [{ uri: { fsPath: "/test/path" } }],
88
getWorkspaceFolder: vi.fn().mockReturnValue({ uri: { fsPath: "/test/path" } }),
9+
createFileSystemWatcher: () => ({
10+
onDidCreate: () => ({ dispose: () => {} }),
11+
onDidChange: () => ({ dispose: () => {} }),
12+
onDidDelete: () => ({ dispose: () => {} }),
13+
dispose: () => {},
14+
}),
915
},
1016
window: {
1117
activeTextEditor: undefined,
18+
createTextEditorDecorationType: () => ({ dispose: () => {} }),
19+
createOutputChannel: () => ({
20+
appendLine: () => {},
21+
show: () => {},
22+
dispose: () => {},
23+
}),
24+
tabGroups: { all: [] },
25+
},
26+
RelativePattern: class {
27+
base: any
28+
pattern: any
29+
constructor(base: any, pattern: any) {
30+
this.base = base
31+
this.pattern = pattern
32+
}
1233
},
1334
EventEmitter: vi.fn().mockImplementation(() => ({
1435
event: vi.fn(),

0 commit comments

Comments
 (0)