Skip to content

Commit 9fa41b7

Browse files
edelaunaedelauna
committed
test(e2e): address mutating tool review feedback
1 parent 009cdd4 commit 9fa41b7

6 files changed

Lines changed: 198 additions & 246 deletions

File tree

‎apps/vscode-e2e/src/fixtures/apply-diff.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export function addApplyDiffResultFixtures(mock: InstanceType<typeof LLMock>) {
2525
},
2626
{
2727
toolCallId: "call_apply_diff_error_001",
28-
result: "Attempted `apply_diff` on `apply-diff-tool-fixture/error-handling.txt`, but the search text was not found so the file was left unchanged.",
28+
result: "The apply_diff operation on `apply-diff-tool-fixture/error-handling.txt` was rejected — the search content did not match any content in the file, so it was not modified.",
2929
id: "call_apply_diff_error_002",
3030
},
3131
{

‎apps/vscode-e2e/src/suite/tools/apply-diff.test.ts‎

Lines changed: 72 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as vscode from "vscode"
55

66
import { RooCodeEventName, type ClineMessage } from "@roo-code/types"
77

8-
import { waitFor, sleep } from "../utils"
8+
import { waitUntilCompleted, sleep } from "../utils"
99
import { setDefaultSuiteTimeout } from "../test-utils"
1010

1111
const TEST_DIR_NAME = "apply-diff-tool-fixture"
@@ -104,11 +104,6 @@ suite("Roo Code apply_diff Tool", function () {
104104

105105
let workspaceDir: string
106106
let testDir: string
107-
const toolApprovalHandler = ({ message }: { message: ClineMessage }) => {
108-
if (message.type === "ask" && message.ask === "tool") {
109-
void globalThis.api.approveCurrentAsk()
110-
}
111-
}
112107

113108
suiteSetup(async () => {
114109
const aimockUrl = process.env.AIMOCK_URL
@@ -120,7 +115,6 @@ suite("Roo Code apply_diff Tool", function () {
120115
openRouterModelId: "anthropic/claude-sonnet-4.5",
121116
...(aimockUrl && { openRouterBaseUrl: `${aimockUrl}/v1` }),
122117
})
123-
globalThis.api.on(RooCodeEventName.Message, toolApprovalHandler)
124118

125119
const workspaceFolders = vscode.workspace.workspaceFolders
126120
if (!workspaceFolders || workspaceFolders.length === 0) {
@@ -147,7 +141,6 @@ suite("Roo Code apply_diff Tool", function () {
147141
openRouterModelId: "openai/gpt-4.1",
148142
...(aimockUrl && { openRouterBaseUrl: `${aimockUrl}/v1` }),
149143
})
150-
globalThis.api.off(RooCodeEventName.Message, toolApprovalHandler)
151144

152145
await fs.rm(testDir, { recursive: true, force: true })
153146
})
@@ -182,7 +175,6 @@ suite("Roo Code apply_diff Tool", function () {
182175
test("Should apply diff to modify existing file content", async function () {
183176
const api = globalThis.api
184177
const messages: ClineMessage[] = []
185-
let taskCompleted = false
186178
let errorOccurred: string | null = null
187179

188180
const messageHandler = ({ message }: { message: ClineMessage }) => {
@@ -193,27 +185,22 @@ suite("Roo Code apply_diff Tool", function () {
193185
}
194186
api.on(RooCodeEventName.Message, messageHandler)
195187

196-
const taskCompletedHandler = (id: string) => {
197-
if (id === taskId) {
198-
taskCompleted = true
199-
}
200-
}
201-
api.on(RooCodeEventName.TaskCompleted, taskCompletedHandler)
202-
203-
let taskId: string
204188
try {
205-
taskId = await api.startNewTask({
206-
configuration: {
207-
mode: "code",
208-
autoApprovalEnabled: true,
209-
alwaysAllowWrite: true,
210-
alwaysAllowReadOnly: true,
211-
alwaysAllowReadOnlyOutsideWorkspace: true,
212-
},
213-
text: "APPLY_DIFF_SIMPLE_SMOKE",
189+
await waitUntilCompleted({
190+
api,
191+
start: () =>
192+
api.startNewTask({
193+
configuration: {
194+
mode: "code",
195+
autoApprovalEnabled: true,
196+
alwaysAllowWrite: true,
197+
alwaysAllowReadOnly: true,
198+
alwaysAllowReadOnlyOutsideWorkspace: true,
199+
},
200+
text: "APPLY_DIFF_SIMPLE_SMOKE",
201+
}),
202+
timeout: 60_000,
214203
})
215-
216-
await waitFor(() => taskCompleted, { timeout: 60_000 })
217204
await sleep(1_000)
218205

219206
assert.strictEqual(errorOccurred, null, `Error occurred: ${errorOccurred}`)
@@ -237,14 +224,12 @@ suite("Roo Code apply_diff Tool", function () {
237224
assert.ok(completionMessage, "AI should have acknowledged the updated file content")
238225
} finally {
239226
api.off(RooCodeEventName.Message, messageHandler)
240-
api.off(RooCodeEventName.TaskCompleted, taskCompletedHandler)
241227
}
242228
})
243229

244230
test("Should apply multiple search/replace blocks in single diff", async function () {
245231
const api = globalThis.api
246232
const messages: ClineMessage[] = []
247-
let taskCompleted = false
248233
let errorOccurred: string | null = null
249234

250235
const messageHandler = ({ message }: { message: ClineMessage }) => {
@@ -255,27 +240,22 @@ suite("Roo Code apply_diff Tool", function () {
255240
}
256241
api.on(RooCodeEventName.Message, messageHandler)
257242

258-
const taskCompletedHandler = (id: string) => {
259-
if (id === taskId) {
260-
taskCompleted = true
261-
}
262-
}
263-
api.on(RooCodeEventName.TaskCompleted, taskCompletedHandler)
264-
265-
let taskId: string
266243
try {
267-
taskId = await api.startNewTask({
268-
configuration: {
269-
mode: "code",
270-
autoApprovalEnabled: true,
271-
alwaysAllowWrite: true,
272-
alwaysAllowReadOnly: true,
273-
alwaysAllowReadOnlyOutsideWorkspace: true,
274-
},
275-
text: "APPLY_DIFF_MULTI_REPLACE_SMOKE",
244+
await waitUntilCompleted({
245+
api,
246+
start: () =>
247+
api.startNewTask({
248+
configuration: {
249+
mode: "code",
250+
autoApprovalEnabled: true,
251+
alwaysAllowWrite: true,
252+
alwaysAllowReadOnly: true,
253+
alwaysAllowReadOnlyOutsideWorkspace: true,
254+
},
255+
text: "APPLY_DIFF_MULTI_REPLACE_SMOKE",
256+
}),
257+
timeout: 60_000,
276258
})
277-
278-
await waitFor(() => taskCompleted, { timeout: 60_000 })
279259
await sleep(1_000)
280260

281261
assert.strictEqual(errorOccurred, null, `Error occurred: ${errorOccurred}`)
@@ -299,14 +279,12 @@ suite("Roo Code apply_diff Tool", function () {
299279
assert.ok(completionMessage, "AI should have acknowledged the multiple replacements")
300280
} finally {
301281
api.off(RooCodeEventName.Message, messageHandler)
302-
api.off(RooCodeEventName.TaskCompleted, taskCompletedHandler)
303282
}
304283
})
305284

306285
test("Should handle apply_diff with line number hints", async function () {
307286
const api = globalThis.api
308287
const messages: ClineMessage[] = []
309-
let taskCompleted = false
310288
let errorOccurred: string | null = null
311289

312290
const messageHandler = ({ message }: { message: ClineMessage }) => {
@@ -317,27 +295,22 @@ suite("Roo Code apply_diff Tool", function () {
317295
}
318296
api.on(RooCodeEventName.Message, messageHandler)
319297

320-
const taskCompletedHandler = (id: string) => {
321-
if (id === taskId) {
322-
taskCompleted = true
323-
}
324-
}
325-
api.on(RooCodeEventName.TaskCompleted, taskCompletedHandler)
326-
327-
let taskId: string
328298
try {
329-
taskId = await api.startNewTask({
330-
configuration: {
331-
mode: "code",
332-
autoApprovalEnabled: true,
333-
alwaysAllowWrite: true,
334-
alwaysAllowReadOnly: true,
335-
alwaysAllowReadOnlyOutsideWorkspace: true,
336-
},
337-
text: "APPLY_DIFF_LINE_HINTS_SMOKE",
299+
await waitUntilCompleted({
300+
api,
301+
start: () =>
302+
api.startNewTask({
303+
configuration: {
304+
mode: "code",
305+
autoApprovalEnabled: true,
306+
alwaysAllowWrite: true,
307+
alwaysAllowReadOnly: true,
308+
alwaysAllowReadOnlyOutsideWorkspace: true,
309+
},
310+
text: "APPLY_DIFF_LINE_HINTS_SMOKE",
311+
}),
312+
timeout: 60_000,
338313
})
339-
340-
await waitFor(() => taskCompleted, { timeout: 60_000 })
341314
await sleep(1_000)
342315

343316
assert.strictEqual(errorOccurred, null, `Error occurred: ${errorOccurred}`)
@@ -358,41 +331,34 @@ suite("Roo Code apply_diff Tool", function () {
358331
assert.ok(completionMessage, "AI should have acknowledged the targeted change")
359332
} finally {
360333
api.off(RooCodeEventName.Message, messageHandler)
361-
api.off(RooCodeEventName.TaskCompleted, taskCompletedHandler)
362334
}
363335
})
364336

365337
test("Should handle apply_diff errors gracefully", async function () {
366338
const api = globalThis.api
367339
const messages: ClineMessage[] = []
368-
let taskCompleted = false
369340

370341
const messageHandler = ({ message }: { message: ClineMessage }) => {
371342
messages.push(message)
372343
}
373344
api.on(RooCodeEventName.Message, messageHandler)
374345

375-
const taskCompletedHandler = (id: string) => {
376-
if (id === taskId) {
377-
taskCompleted = true
378-
}
379-
}
380-
api.on(RooCodeEventName.TaskCompleted, taskCompletedHandler)
381-
382-
let taskId: string
383346
try {
384-
taskId = await api.startNewTask({
385-
configuration: {
386-
mode: "code",
387-
autoApprovalEnabled: true,
388-
alwaysAllowWrite: true,
389-
alwaysAllowReadOnly: true,
390-
alwaysAllowReadOnlyOutsideWorkspace: true,
391-
},
392-
text: "APPLY_DIFF_ERROR_SMOKE",
347+
await waitUntilCompleted({
348+
api,
349+
start: () =>
350+
api.startNewTask({
351+
configuration: {
352+
mode: "code",
353+
autoApprovalEnabled: true,
354+
alwaysAllowWrite: true,
355+
alwaysAllowReadOnly: true,
356+
alwaysAllowReadOnlyOutsideWorkspace: true,
357+
},
358+
text: "APPLY_DIFF_ERROR_SMOKE",
359+
}),
360+
timeout: 60_000,
393361
})
394-
395-
await waitFor(() => taskCompleted, { timeout: 60_000 })
396362
await sleep(1_000)
397363

398364
const actualContent = await fs.readFile(
@@ -409,19 +375,17 @@ suite("Roo Code apply_diff Tool", function () {
409375
(message) =>
410376
message.type === "say" &&
411377
(message.say === "completion_result" || message.say === "text") &&
412-
message.text?.includes("left unchanged"),
378+
message.text?.includes("did not match"),
413379
)
414380
assert.ok(completionMessage, "AI should have acknowledged the graceful apply_diff failure")
415381
} finally {
416382
api.off(RooCodeEventName.Message, messageHandler)
417-
api.off(RooCodeEventName.TaskCompleted, taskCompletedHandler)
418383
}
419384
})
420385

421386
test("Should apply multiple search/replace blocks to edit two separate functions", async function () {
422387
const api = globalThis.api
423388
const messages: ClineMessage[] = []
424-
let taskCompleted = false
425389
let errorOccurred: string | null = null
426390

427391
const messageHandler = ({ message }: { message: ClineMessage }) => {
@@ -432,27 +396,22 @@ suite("Roo Code apply_diff Tool", function () {
432396
}
433397
api.on(RooCodeEventName.Message, messageHandler)
434398

435-
const taskCompletedHandler = (id: string) => {
436-
if (id === taskId) {
437-
taskCompleted = true
438-
}
439-
}
440-
api.on(RooCodeEventName.TaskCompleted, taskCompletedHandler)
441-
442-
let taskId: string
443399
try {
444-
taskId = await api.startNewTask({
445-
configuration: {
446-
mode: "code",
447-
autoApprovalEnabled: true,
448-
alwaysAllowWrite: true,
449-
alwaysAllowReadOnly: true,
450-
alwaysAllowReadOnlyOutsideWorkspace: true,
451-
},
452-
text: "APPLY_DIFF_MULTI_BLOCK_SMOKE",
400+
await waitUntilCompleted({
401+
api,
402+
start: () =>
403+
api.startNewTask({
404+
configuration: {
405+
mode: "code",
406+
autoApprovalEnabled: true,
407+
alwaysAllowWrite: true,
408+
alwaysAllowReadOnly: true,
409+
alwaysAllowReadOnlyOutsideWorkspace: true,
410+
},
411+
text: "APPLY_DIFF_MULTI_BLOCK_SMOKE",
412+
}),
413+
timeout: 60_000,
453414
})
454-
455-
await waitFor(() => taskCompleted, { timeout: 60_000 })
456415
await sleep(1_000)
457416

458417
assert.strictEqual(errorOccurred, null, `Error occurred: ${errorOccurred}`)
@@ -476,7 +435,6 @@ suite("Roo Code apply_diff Tool", function () {
476435
assert.ok(completionMessage, "AI should have acknowledged the multi-block apply_diff update")
477436
} finally {
478437
api.off(RooCodeEventName.Message, messageHandler)
479-
api.off(RooCodeEventName.TaskCompleted, taskCompletedHandler)
480438
}
481439
})
482440
})

0 commit comments

Comments
 (0)