mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
* feat: add configurable delay for Go diagnostics to prevent premature error reporting - Add diagnosticsDelayMs setting (default: 2000ms) to allow linters time to process - Add diagnosticsEnabled setting to optionally disable diagnostic checking entirely - Update DiffViewProvider.saveChanges() to use configurable delay before checking diagnostics - Update all tool files (writeToFile, searchAndReplace, insertContent, applyDiff, multiApplyDiff) to pass diagnostic settings - Add comprehensive tests for new diagnostic functionality - Fixes issue where Go diagnostics errors were submitted to LLM before linter could clean up unused imports Resolves #5859 * fix: add missing TypeScript type definitions for diagnostic settings - Add diagnosticsDelayMs and diagnosticsEnabled to globalSettingsSchema - Include properties in ExtensionState Pick type - Add default values to EVALS_SETTINGS - Fix VSCode mock to include DiagnosticSeverity for tests - Resolves compilation errors in ClineProvider and webviewMessageHandler * fix: update test mocks to support diagnostic settings in tool tests - Add providerRef mock to insertContentTool and writeToFileTool tests - Update mocks to include diagnosticsEnabled and diagnosticsDelayMs settings - Fix test expectations to match new implementation with diagnostic configuration - Resolves failing unit tests for insertContentTool.spec.ts and writeToFileTool.spec.ts * fix: remove package-lock.json file (project uses pnpm) * refactor: use existing writeDelayMs instead of diagnosticsDelayMs - Remove diagnosticsDelayMs setting in favor of existing writeDelayMs - Add min(0) validation for writeDelayMs in global settings schema - Add error handling around delay function calls in DiffViewProvider - Create DEFAULT_WRITE_DELAY_MS constant (1000ms) to replace repeated defaults - Update all tool files to pass writeDelayMs instead of diagnosticsDelayMs - Remove diagnosticsDelayMs from webview message handlers and types - Update test files to use writeDelayMs instead of diagnosticsDelayMs This refactoring consolidates diagnostic delay functionality to use the existing writeDelayMs setting as requested in PR feedback. * fix: resolve failing unit tests and TypeScript compilation errors - Fix DiffViewProvider test to expect correct default delay (1000ms instead of 2000ms) - Fix TypeScript type errors in ClineProvider test mock state object - Correct terminalPowershellCounter and terminalZdotdir types to boolean - Fix pinnedApiConfigs type from array to Record<string, boolean> * fix: remove unrelated changes from ClineProvider.spec.ts - Removed extensive unrelated property additions to mock state - Kept only diagnosticsEnabled property which is related to Go diagnostics delay feature - Removed unused DEFAULT_WRITE_DELAY_MS import - Restored original structure and organization of mock state object This addresses the feedback to remove unrelated changes while preserving the necessary diagnostic functionality for the Go diagnostics delay feature. * refactor: move DEFAULT_WRITE_DELAY_MS to packages/types/src/global-settings.ts - Move DEFAULT_WRITE_DELAY_MS constant from src/shared/constants.ts to packages/types/src/global-settings.ts - Update all import statements in affected files to use @roo-code/types - Delete src/shared/constants.ts file as it is no longer needed - Files updated: - src/integrations/editor/DiffViewProvider.ts - src/core/webview/ClineProvider.ts - src/core/tools/multiApplyDiffTool.ts - src/core/tools/applyDiffTool.ts - src/core/tools/searchAndReplaceTool.ts - src/core/tools/insertContentTool.ts - src/core/tools/writeToFileTool.ts --------- Co-authored-by: Roo Code <roomote@roocode.com>
421 lines
12 KiB
TypeScript
421 lines
12 KiB
TypeScript
import { DiffViewProvider, DIFF_VIEW_URI_SCHEME, DIFF_VIEW_LABEL_CHANGES } from "../DiffViewProvider"
|
|
import * as vscode from "vscode"
|
|
import * as path from "path"
|
|
import delay from "delay"
|
|
|
|
// Mock delay
|
|
vi.mock("delay", () => ({
|
|
default: vi.fn().mockResolvedValue(undefined),
|
|
}))
|
|
|
|
// Mock fs/promises
|
|
vi.mock("fs/promises", () => ({
|
|
readFile: vi.fn().mockResolvedValue("file content"),
|
|
writeFile: vi.fn().mockResolvedValue(undefined),
|
|
}))
|
|
|
|
// Mock utils
|
|
vi.mock("../../../utils/fs", () => ({
|
|
createDirectoriesForFile: vi.fn().mockResolvedValue([]),
|
|
}))
|
|
|
|
// Mock path
|
|
vi.mock("path", () => ({
|
|
resolve: vi.fn((cwd, relPath) => `${cwd}/${relPath}`),
|
|
basename: vi.fn((path) => path.split("/").pop()),
|
|
}))
|
|
|
|
// Mock vscode
|
|
vi.mock("vscode", () => ({
|
|
workspace: {
|
|
applyEdit: vi.fn(),
|
|
onDidOpenTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
|
|
textDocuments: [],
|
|
fs: {
|
|
stat: vi.fn(),
|
|
},
|
|
},
|
|
window: {
|
|
createTextEditorDecorationType: vi.fn(),
|
|
showTextDocument: vi.fn(),
|
|
onDidChangeVisibleTextEditors: vi.fn(() => ({ dispose: vi.fn() })),
|
|
tabGroups: {
|
|
all: [],
|
|
close: vi.fn(),
|
|
},
|
|
visibleTextEditors: [],
|
|
},
|
|
commands: {
|
|
executeCommand: vi.fn(),
|
|
},
|
|
languages: {
|
|
getDiagnostics: vi.fn(() => []),
|
|
},
|
|
DiagnosticSeverity: {
|
|
Error: 0,
|
|
Warning: 1,
|
|
Information: 2,
|
|
Hint: 3,
|
|
},
|
|
WorkspaceEdit: vi.fn().mockImplementation(() => ({
|
|
replace: vi.fn(),
|
|
delete: vi.fn(),
|
|
})),
|
|
ViewColumn: {
|
|
Active: 1,
|
|
Beside: 2,
|
|
One: 1,
|
|
Two: 2,
|
|
Three: 3,
|
|
Four: 4,
|
|
Five: 5,
|
|
Six: 6,
|
|
Seven: 7,
|
|
Eight: 8,
|
|
Nine: 9,
|
|
},
|
|
Range: vi.fn(),
|
|
Position: vi.fn(),
|
|
Selection: vi.fn(),
|
|
TextEditorRevealType: {
|
|
InCenter: 2,
|
|
},
|
|
TabInputTextDiff: class TabInputTextDiff {},
|
|
Uri: {
|
|
file: vi.fn((path) => ({ fsPath: path })),
|
|
parse: vi.fn((uri) => ({ with: vi.fn(() => ({})) })),
|
|
},
|
|
}))
|
|
|
|
// Mock DecorationController
|
|
vi.mock("../DecorationController", () => ({
|
|
DecorationController: vi.fn().mockImplementation(() => ({
|
|
setActiveLine: vi.fn(),
|
|
updateOverlayAfterLine: vi.fn(),
|
|
addLines: vi.fn(),
|
|
clear: vi.fn(),
|
|
})),
|
|
}))
|
|
|
|
describe("DiffViewProvider", () => {
|
|
let diffViewProvider: DiffViewProvider
|
|
const mockCwd = "/mock/cwd"
|
|
let mockWorkspaceEdit: { replace: any; delete: any }
|
|
|
|
beforeEach(() => {
|
|
vi.clearAllMocks()
|
|
mockWorkspaceEdit = {
|
|
replace: vi.fn(),
|
|
delete: vi.fn(),
|
|
}
|
|
vi.mocked(vscode.WorkspaceEdit).mockImplementation(() => mockWorkspaceEdit as any)
|
|
|
|
diffViewProvider = new DiffViewProvider(mockCwd)
|
|
// Mock the necessary properties and methods
|
|
;(diffViewProvider as any).relPath = "test.txt"
|
|
;(diffViewProvider as any).activeDiffEditor = {
|
|
document: {
|
|
uri: { fsPath: `${mockCwd}/test.txt` },
|
|
getText: vi.fn(),
|
|
lineCount: 10,
|
|
},
|
|
selection: {
|
|
active: { line: 0, character: 0 },
|
|
anchor: { line: 0, character: 0 },
|
|
},
|
|
edit: vi.fn().mockResolvedValue(true),
|
|
revealRange: vi.fn(),
|
|
}
|
|
;(diffViewProvider as any).activeLineController = { setActiveLine: vi.fn(), clear: vi.fn() }
|
|
;(diffViewProvider as any).fadedOverlayController = {
|
|
updateOverlayAfterLine: vi.fn(),
|
|
addLines: vi.fn(),
|
|
clear: vi.fn(),
|
|
}
|
|
})
|
|
|
|
describe("update method", () => {
|
|
it("should preserve empty last line when original content has one", async () => {
|
|
;(diffViewProvider as any).originalContent = "Original content\n"
|
|
await diffViewProvider.update("New content", true)
|
|
|
|
expect(mockWorkspaceEdit.replace).toHaveBeenCalledWith(
|
|
expect.anything(),
|
|
expect.anything(),
|
|
"New content\n",
|
|
)
|
|
})
|
|
|
|
it("should not add extra newline when accumulated content already ends with one", async () => {
|
|
;(diffViewProvider as any).originalContent = "Original content\n"
|
|
await diffViewProvider.update("New content\n", true)
|
|
|
|
expect(mockWorkspaceEdit.replace).toHaveBeenCalledWith(
|
|
expect.anything(),
|
|
expect.anything(),
|
|
"New content\n",
|
|
)
|
|
})
|
|
|
|
it("should not add newline when original content does not end with one", async () => {
|
|
;(diffViewProvider as any).originalContent = "Original content"
|
|
await diffViewProvider.update("New content", true)
|
|
|
|
expect(mockWorkspaceEdit.replace).toHaveBeenCalledWith(expect.anything(), expect.anything(), "New content")
|
|
})
|
|
})
|
|
|
|
describe("open method", () => {
|
|
it("should pre-open file as text document before executing diff command", async () => {
|
|
// Setup
|
|
const mockEditor = {
|
|
document: {
|
|
uri: { fsPath: `${mockCwd}/test.md` },
|
|
getText: vi.fn().mockReturnValue(""),
|
|
lineCount: 0,
|
|
},
|
|
selection: {
|
|
active: { line: 0, character: 0 },
|
|
anchor: { line: 0, character: 0 },
|
|
},
|
|
edit: vi.fn().mockResolvedValue(true),
|
|
revealRange: vi.fn(),
|
|
}
|
|
|
|
// Track the order of calls
|
|
const callOrder: string[] = []
|
|
|
|
// Mock showTextDocument to track when it's called
|
|
vi.mocked(vscode.window.showTextDocument).mockImplementation(async (uri, options) => {
|
|
callOrder.push("showTextDocument")
|
|
expect(options).toEqual({ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true })
|
|
return mockEditor as any
|
|
})
|
|
|
|
// Mock executeCommand to track when it's called
|
|
vi.mocked(vscode.commands.executeCommand).mockImplementation(async (command) => {
|
|
callOrder.push("executeCommand")
|
|
expect(command).toBe("vscode.diff")
|
|
return undefined
|
|
})
|
|
|
|
// Mock workspace.onDidOpenTextDocument to trigger immediately
|
|
vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => {
|
|
// Trigger the callback immediately with the document
|
|
setTimeout(() => {
|
|
callback({ uri: { fsPath: `${mockCwd}/test.md` } } as any)
|
|
}, 0)
|
|
return { dispose: vi.fn() }
|
|
})
|
|
|
|
// Mock window.visibleTextEditors to return our editor
|
|
vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any]
|
|
|
|
// Set up for file
|
|
;(diffViewProvider as any).editType = "modify"
|
|
|
|
// Execute open
|
|
await diffViewProvider.open("test.md")
|
|
|
|
// Verify that showTextDocument was called before executeCommand
|
|
expect(callOrder).toEqual(["showTextDocument", "executeCommand"])
|
|
|
|
// Verify that showTextDocument was called with preview: false and preserveFocus: true
|
|
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
|
|
expect.objectContaining({ fsPath: `${mockCwd}/test.md` }),
|
|
{ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true },
|
|
)
|
|
|
|
// Verify that the diff command was executed
|
|
expect(vscode.commands.executeCommand).toHaveBeenCalledWith(
|
|
"vscode.diff",
|
|
expect.any(Object),
|
|
expect.any(Object),
|
|
`test.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`,
|
|
{ preserveFocus: true },
|
|
)
|
|
})
|
|
|
|
it("should handle showTextDocument failure", async () => {
|
|
// Mock showTextDocument to fail
|
|
vi.mocked(vscode.window.showTextDocument).mockRejectedValue(new Error("Cannot open file"))
|
|
|
|
// Mock workspace.onDidOpenTextDocument
|
|
vi.mocked(vscode.workspace.onDidOpenTextDocument).mockReturnValue({ dispose: vi.fn() })
|
|
|
|
// Mock window.onDidChangeVisibleTextEditors
|
|
vi.mocked(vscode.window.onDidChangeVisibleTextEditors).mockReturnValue({ dispose: vi.fn() })
|
|
|
|
// Set up for file
|
|
;(diffViewProvider as any).editType = "modify"
|
|
|
|
// Try to open and expect rejection
|
|
await expect(diffViewProvider.open("test.md")).rejects.toThrow(
|
|
"Failed to execute diff command for /mock/cwd/test.md: Cannot open file",
|
|
)
|
|
})
|
|
})
|
|
|
|
describe("closeAllDiffViews method", () => {
|
|
it("should close diff views including those identified by label", async () => {
|
|
// Mock tab groups with various types of tabs
|
|
const mockTabs = [
|
|
// Normal diff view
|
|
{
|
|
input: {
|
|
constructor: { name: "TabInputTextDiff" },
|
|
original: { scheme: DIFF_VIEW_URI_SCHEME },
|
|
modified: { fsPath: "/test/file1.ts" },
|
|
},
|
|
label: `file1.ts: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`,
|
|
isDirty: false,
|
|
},
|
|
// Diff view identified by label (for pre-opened files)
|
|
{
|
|
input: {
|
|
constructor: { name: "TabInputTextDiff" },
|
|
original: { scheme: "file" }, // Different scheme due to pre-opening
|
|
modified: { fsPath: "/test/file2.md" },
|
|
},
|
|
label: `file2.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`,
|
|
isDirty: false,
|
|
},
|
|
// Regular file tab (should not be closed)
|
|
{
|
|
input: {
|
|
constructor: { name: "TabInputText" },
|
|
uri: { fsPath: "/test/file3.js" },
|
|
},
|
|
label: "file3.js",
|
|
isDirty: false,
|
|
},
|
|
// Dirty diff view (should not be closed)
|
|
{
|
|
input: {
|
|
constructor: { name: "TabInputTextDiff" },
|
|
original: { scheme: DIFF_VIEW_URI_SCHEME },
|
|
modified: { fsPath: "/test/file4.ts" },
|
|
},
|
|
label: `file4.ts: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`,
|
|
isDirty: true,
|
|
},
|
|
]
|
|
|
|
// Make tabs appear as TabInputTextDiff instances
|
|
mockTabs.forEach((tab) => {
|
|
if (tab.input.constructor.name === "TabInputTextDiff") {
|
|
Object.setPrototypeOf(tab.input, vscode.TabInputTextDiff.prototype)
|
|
}
|
|
})
|
|
|
|
// Mock the tabGroups getter
|
|
Object.defineProperty(vscode.window.tabGroups, "all", {
|
|
get: () => [
|
|
{
|
|
tabs: mockTabs as any,
|
|
},
|
|
],
|
|
configurable: true,
|
|
})
|
|
|
|
const closedTabs: any[] = []
|
|
vi.mocked(vscode.window.tabGroups.close).mockImplementation((tab) => {
|
|
closedTabs.push(tab)
|
|
return Promise.resolve(true)
|
|
})
|
|
|
|
// Execute closeAllDiffViews
|
|
await (diffViewProvider as any).closeAllDiffViews()
|
|
|
|
// Verify that only the appropriate tabs were closed
|
|
expect(closedTabs).toHaveLength(2)
|
|
expect(closedTabs[0].label).toBe(`file1.ts: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`)
|
|
expect(closedTabs[1].label).toBe(`file2.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`)
|
|
|
|
// Verify that the regular file and dirty diff were not closed
|
|
expect(closedTabs.find((t) => t.label === "file3.js")).toBeUndefined()
|
|
expect(
|
|
closedTabs.find((t) => t.label === `file4.ts: ${DIFF_VIEW_LABEL_CHANGES} (Editable)` && t.isDirty),
|
|
).toBeUndefined()
|
|
})
|
|
})
|
|
|
|
describe("saveChanges method with diagnostic settings", () => {
|
|
beforeEach(() => {
|
|
// Setup common mocks for saveChanges tests
|
|
;(diffViewProvider as any).relPath = "test.ts"
|
|
;(diffViewProvider as any).newContent = "new content"
|
|
;(diffViewProvider as any).activeDiffEditor = {
|
|
document: {
|
|
getText: vi.fn().mockReturnValue("new content"),
|
|
isDirty: false,
|
|
save: vi.fn().mockResolvedValue(undefined),
|
|
},
|
|
}
|
|
;(diffViewProvider as any).preDiagnostics = []
|
|
|
|
// Mock vscode functions
|
|
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
|
|
vi.mocked(vscode.languages.getDiagnostics).mockReturnValue([])
|
|
})
|
|
|
|
it("should apply diagnostic delay when diagnosticsEnabled is true", async () => {
|
|
const mockDelay = vi.mocked(delay)
|
|
mockDelay.mockClear()
|
|
|
|
// Mock closeAllDiffViews
|
|
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
|
|
|
|
const result = await diffViewProvider.saveChanges(true, 3000)
|
|
|
|
// Verify delay was called with correct duration
|
|
expect(mockDelay).toHaveBeenCalledWith(3000)
|
|
expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
|
|
expect(result.newProblemsMessage).toBe("")
|
|
})
|
|
|
|
it("should skip diagnostics when diagnosticsEnabled is false", async () => {
|
|
const mockDelay = vi.mocked(delay)
|
|
mockDelay.mockClear()
|
|
|
|
// Mock closeAllDiffViews
|
|
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
|
|
|
|
const result = await diffViewProvider.saveChanges(false, 2000)
|
|
|
|
// Verify delay was NOT called and diagnostics were NOT checked
|
|
expect(mockDelay).not.toHaveBeenCalled()
|
|
expect(vscode.languages.getDiagnostics).not.toHaveBeenCalled()
|
|
expect(result.newProblemsMessage).toBe("")
|
|
})
|
|
|
|
it("should use default values when no parameters provided", async () => {
|
|
const mockDelay = vi.mocked(delay)
|
|
mockDelay.mockClear()
|
|
|
|
// Mock closeAllDiffViews
|
|
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
|
|
|
|
const result = await diffViewProvider.saveChanges()
|
|
|
|
// Verify default behavior (enabled=true, delay=2000ms)
|
|
expect(mockDelay).toHaveBeenCalledWith(1000)
|
|
expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
|
|
expect(result.newProblemsMessage).toBe("")
|
|
})
|
|
|
|
it("should handle custom delay values", async () => {
|
|
const mockDelay = vi.mocked(delay)
|
|
mockDelay.mockClear()
|
|
|
|
// Mock closeAllDiffViews
|
|
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
|
|
|
|
const result = await diffViewProvider.saveChanges(true, 5000)
|
|
|
|
// Verify custom delay was used
|
|
expect(mockDelay).toHaveBeenCalledWith(5000)
|
|
expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
|
|
})
|
|
})
|
|
})
|