scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-09-07 08:26:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: update applyDiffTool experiment tests for new routing architecture

Browse source
This commit is contained in:
Hannes Rudolph 2025-12-15 11:38:46 -07:00
parent 7e52525523
commit e3cb501a67
1 changed files with 93 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

149
src/core/tools/__tests__/applyDiffTool.experiment.spec.ts
View file

@ -8,17 +8,21 @@ vi.mock("vscode", () => ({
},
}))
// Mock the ApplyDiffTool module
vi.mock("../ApplyDiffTool", () => ({
applyDiffTool: {
handle: vi.fn(),
},
}))
// Import after mocking to get the mocked version
import { applyDiffTool as multiApplyDiffTool } from "../MultiApplyDiffTool"
import { applyDiffTool as applyDiffToolClass } from "../ApplyDiffTool"
/**
* These tests verify that multiApplyDiffTool properly handles multi-file operations.
*
* NOTE: Routing between single-file and multi-file tools is now done in presentAssistantMessage.ts
* based on nativeArgs format. multiApplyDiffTool is responsible for:
* - Handling XML args format (multi-file XML protocol)
* - Handling nativeArgs.files format (multi-file native protocol)
* - Handling legacy path/diff params (single file, XML protocol)
*
* The routing logic tests (when to use applyDiffTool vs multiApplyDiffTool) should be
* tested in presentAssistantMessage.spec.ts or integration tests.
*/
describe("applyDiffTool experiment routing", () => {
let mockCline: any
let mockBlock: any
@ -68,9 +72,22 @@ describe("applyDiffTool experiment routing", () => {
}),
},
processQueuedMessages: vi.fn(),
consecutiveMistakeCount: 0,
recordToolError: vi.fn(),
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
// Required for file access validation
rooIgnoreController: {
validateAccess: vi.fn().mockReturnValue(true),
},
rooProtectedController: {
isWriteProtected: vi.fn().mockReturnValue(false),
},
say: vi.fn().mockResolvedValue(undefined),
} as any
mockBlock = {
type: "tool_use",
name: "apply_diff",
params: {
path: "test.ts",
diff: "test diff",
@ -84,16 +101,18 @@ describe("applyDiffTool experiment routing", () => {
mockRemoveClosingTag = vi.fn((tag, value) => value)
})
it("should use legacy tool when MULTI_FILE_APPLY_DIFF experiment is disabled", async () => {
it("should handle legacy params directly when experiment is disabled", async () => {
// With new architecture, multiApplyDiffTool handles the request directly
// when called with legacy params. Routing to applyDiffTool (single-file)
// is now done in presentAssistantMessage.ts BEFORE calling this function.
mockProvider.getState.mockResolvedValue({
experiments: {
[EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF]: false,
},
})
// Mock the class-based tool to resolve successfully
;(applyDiffToolClass.handle as any).mockResolvedValue(undefined)
// This will result in an error because the file doesn't exist,
// but it verifies the function processes the request directly
await multiApplyDiffTool(
mockCline,
mockBlock,
@ -103,21 +122,15 @@ describe("applyDiffTool experiment routing", () => {
mockRemoveClosingTag,
)
expect(applyDiffToolClass.handle).toHaveBeenCalledWith(mockCline, mockBlock, {
askApproval: mockAskApproval,
handleError: mockHandleError,
pushToolResult: mockPushToolResult,
removeClosingTag: mockRemoveClosingTag,
toolProtocol: "xml",
})
// Function should process the request - it will push a result (error about missing file)
expect(mockPushToolResult).toHaveBeenCalled()
})
it("should use legacy tool when experiments are not defined", async () => {
it("should handle legacy params directly when experiments are not defined", async () => {
mockProvider.getState.mockResolvedValue({})
// Mock the class-based tool to resolve successfully
;(applyDiffToolClass.handle as any).mockResolvedValue(undefined)
// This will result in an error because the file doesn't exist,
// but it verifies the function processes the request directly
await multiApplyDiffTool(
mockCline,
mockBlock,
@ -127,24 +140,19 @@ describe("applyDiffTool experiment routing", () => {
mockRemoveClosingTag,
)
expect(applyDiffToolClass.handle).toHaveBeenCalledWith(mockCline, mockBlock, {
askApproval: mockAskApproval,
handleError: mockHandleError,
pushToolResult: mockPushToolResult,
removeClosingTag: mockRemoveClosingTag,
toolProtocol: "xml",
})
// Function should process the request - it will push a result (error about missing file)
expect(mockPushToolResult).toHaveBeenCalled()
})
it("should use multi-file tool when MULTI_FILE_APPLY_DIFF experiment is enabled and using XML protocol", async () => {
it("should handle multi-file operations when MULTI_FILE_APPLY_DIFF experiment is enabled", async () => {
mockProvider.getState.mockResolvedValue({
experiments: {
[EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF]: true,
},
})
// Mock the new tool behavior - it should continue with the multi-file implementation
// Since we're not mocking the entire function, we'll just verify it doesn't call the class-based tool
// This will result in an error because the file doesn't exist,
// but it verifies the function processes the request directly
await multiApplyDiffTool(
mockCline,
mockBlock,
@ -154,45 +162,74 @@ describe("applyDiffTool experiment routing", () => {
mockRemoveClosingTag,
)
expect(applyDiffToolClass.handle).not.toHaveBeenCalled()
// Function should process the request directly
expect(mockPushToolResult).toHaveBeenCalled()
})
it("should use class-based tool when model defaults to native protocol", async () => {
// Update model to support native tools and default to native protocol
mockCline.api.getModel = vi.fn().mockReturnValue({
id: "test-model",
info: {
maxTokens: 4096,
contextWindow: 128000,
supportsPromptCache: false,
supportsNativeTools: true, // Model supports native tools
defaultToolProtocol: "native", // Model defaults to native protocol
},
})
it("should handle native multi-file format (nativeArgs.files)", async () => {
// Test that multiApplyDiffTool properly handles native multi-file format
mockProvider.getState.mockResolvedValue({
experiments: {
[EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF]: true,
},
})
;(applyDiffToolClass.handle as any).mockResolvedValue(undefined)
const blockWithNativeArgs = {
type: "tool_use" as const,
name: "apply_diff" as const,
params: {},
partial: false,
nativeArgs: {
files: [
{ path: "test1.ts", diff: "test diff 1" },
{ path: "test2.ts", diff: "test diff 2" },
],
},
}
await multiApplyDiffTool(
mockCline,
mockBlock,
blockWithNativeArgs,
mockAskApproval,
mockHandleError,
mockPushToolResult,
mockRemoveClosingTag,
)
// When native protocol is used, should always use class-based tool
expect(applyDiffToolClass.handle).toHaveBeenCalledWith(mockCline, mockBlock, {
askApproval: mockAskApproval,
handleError: mockHandleError,
pushToolResult: mockPushToolResult,
removeClosingTag: mockRemoveClosingTag,
toolProtocol: "native",
// Function should process the multi-file request
expect(mockPushToolResult).toHaveBeenCalled()
})
it("should handle partial messages for native multi-file format", async () => {
mockProvider.getState.mockResolvedValue({
experiments: {
[EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF]: true,
},
})
const mockAsk = vi.fn().mockResolvedValue({})
mockCline.ask = mockAsk
const blockWithNativeArgs = {
type: "tool_use" as const,
name: "apply_diff" as const,
params: {},
partial: true,
nativeArgs: {
files: [{ path: "test1.ts", diff: "partial diff" }],
},
}
await multiApplyDiffTool(
mockCline,
blockWithNativeArgs,
mockAskApproval,
mockHandleError,
mockPushToolResult,
mockRemoveClosingTag,
)
// For partial messages, should call ask with partial=true
expect(mockAsk).toHaveBeenCalledWith("tool", expect.any(String), true)
})
})