Skip to content

Commit 781fa62

Browse files
author
Eric Wheeler
committed
test: add comprehensive tests for read_file maxReadFileLine settings
- Tests behavior when maxReadFileLine is -1 (reads entire file) - Tests behavior when maxReadFileLine >= file length (reads entire file) - Tests behavior when maxReadFileLine is 0 (shows only definitions) - Tests behavior when maxReadFileLine < file length (truncates content) - Verifies exact line counts in responses - Adds helper functions for response validation - Includes DEBUG flag for controlled logging
1 parent 4212e4a commit 781fa62

File tree

1 file changed

+325
-0
lines changed

1 file changed

+325
-0
lines changed
Lines changed: 325 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,325 @@
1+
const DEBUG = false
2+
3+
import * as path from "path"
4+
import { countFileLines } from "../../integrations/misc/line-counter"
5+
import { readLines } from "../../integrations/misc/read-lines"
6+
import { extractTextFromFile, addLineNumbers } from "../../integrations/misc/extract-text"
7+
import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter"
8+
import { isBinaryFile } from "isbinaryfile"
9+
import { ReadFileToolUse } from "../assistant-message"
10+
import { Cline } from "../Cline"
11+
import { ClineProvider } from "../webview/ClineProvider"
12+
13+
// Mock dependencies
14+
jest.mock("../../integrations/misc/line-counter")
15+
jest.mock("../../integrations/misc/read-lines")
16+
jest.mock("../../integrations/misc/extract-text")
17+
jest.mock("../../services/tree-sitter")
18+
jest.mock("isbinaryfile")
19+
jest.mock("../ignore/RooIgnoreController", () => ({
20+
RooIgnoreController: class {
21+
initialize() {
22+
return Promise.resolve()
23+
}
24+
validateAccess(filePath: string) {
25+
return true
26+
}
27+
},
28+
}))
29+
jest.mock("fs/promises", () => ({
30+
mkdir: jest.fn().mockResolvedValue(undefined),
31+
writeFile: jest.fn().mockResolvedValue(undefined),
32+
readFile: jest.fn().mockResolvedValue("{}"),
33+
}))
34+
jest.mock("../../utils/fs", () => ({
35+
fileExistsAtPath: jest.fn().mockReturnValue(true),
36+
}))
37+
38+
// Mock path
39+
jest.mock("path", () => {
40+
const originalPath = jest.requireActual("path")
41+
return {
42+
...originalPath,
43+
resolve: jest.fn().mockImplementation((...args) => args.join("/")),
44+
}
45+
})
46+
47+
describe("read_file tool with maxReadFileLine setting", () => {
48+
// Mock original implementation first to use in tests
49+
const originalCountFileLines = jest.requireActual("../../integrations/misc/line-counter").countFileLines
50+
const originalReadLines = jest.requireActual("../../integrations/misc/read-lines").readLines
51+
const originalExtractTextFromFile = jest.requireActual("../../integrations/misc/extract-text").extractTextFromFile
52+
const originalAddLineNumbers = jest.requireActual("../../integrations/misc/extract-text").addLineNumbers
53+
const originalParseSourceCodeDefinitionsForFile =
54+
jest.requireActual("../../services/tree-sitter").parseSourceCodeDefinitionsForFile
55+
const originalIsBinaryFile = jest.requireActual("isbinaryfile").isBinaryFile
56+
57+
let cline: Cline
58+
let mockProvider: any
59+
const testFilePath = "test/file.txt"
60+
const absoluteFilePath = "/home/ewheeler/src/roo/roo-main/test/file.txt"
61+
const fileContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5"
62+
const numberedFileContent = "1 | Line 1\n2 | Line 2\n3 | Line 3\n4 | Line 4\n5 | Line 5"
63+
const sourceCodeDef = "\n\n# file.txt\n1--5 | Content"
64+
65+
beforeEach(() => {
66+
jest.resetAllMocks()
67+
68+
// Reset mocks to simulate original behavior
69+
;(countFileLines as jest.Mock).mockImplementation(originalCountFileLines)
70+
;(readLines as jest.Mock).mockImplementation(originalReadLines)
71+
;(extractTextFromFile as jest.Mock).mockImplementation(originalExtractTextFromFile)
72+
;(parseSourceCodeDefinitionsForFile as jest.Mock).mockImplementation(originalParseSourceCodeDefinitionsForFile)
73+
;(isBinaryFile as jest.Mock).mockImplementation(originalIsBinaryFile)
74+
75+
// Default mock implementations
76+
;(countFileLines as jest.Mock).mockResolvedValue(5)
77+
;(readLines as jest.Mock).mockResolvedValue(fileContent)
78+
;(extractTextFromFile as jest.Mock).mockResolvedValue(numberedFileContent)
79+
// Use the real addLineNumbers function
80+
;(addLineNumbers as jest.Mock).mockImplementation(originalAddLineNumbers)
81+
;(parseSourceCodeDefinitionsForFile as jest.Mock).mockResolvedValue(sourceCodeDef)
82+
;(isBinaryFile as jest.Mock).mockResolvedValue(false)
83+
84+
// Add spy to debug the readLines calls
85+
const readLinesSpy = jest.spyOn(require("../../integrations/misc/read-lines"), "readLines")
86+
87+
// Mock path.resolve to return a predictable path
88+
;(path.resolve as jest.Mock).mockReturnValue(absoluteFilePath)
89+
90+
// Create mock provider
91+
mockProvider = {
92+
getState: jest.fn(),
93+
deref: jest.fn().mockReturnThis(),
94+
}
95+
96+
// Create a Cline instance with the necessary configuration
97+
cline = new Cline({
98+
provider: mockProvider,
99+
apiConfiguration: { apiProvider: "anthropic" } as any,
100+
task: "Test read_file tool", // Required to satisfy constructor check
101+
startTask: false, // Prevent actual task initialization
102+
})
103+
104+
// Set up the read_file tool use
105+
const readFileToolUse: ReadFileToolUse = {
106+
type: "tool_use",
107+
name: "read_file",
108+
params: {
109+
path: testFilePath,
110+
},
111+
partial: false,
112+
}
113+
114+
// Set up the Cline instance for testing
115+
const clineAny = cline as any
116+
117+
// Set up the required properties for the test
118+
clineAny.assistantMessageContent = [readFileToolUse]
119+
clineAny.currentStreamingContentIndex = 0
120+
clineAny.userMessageContent = []
121+
clineAny.presentAssistantMessageLocked = false
122+
clineAny.didCompleteReadingStream = true
123+
clineAny.didRejectTool = false
124+
clineAny.didAlreadyUseTool = false
125+
126+
// Mock methods that would be called during presentAssistantMessage
127+
clineAny.say = jest.fn().mockResolvedValue(undefined)
128+
clineAny.ask = jest.fn().mockImplementation((type, message) => {
129+
return Promise.resolve({ response: "yesButtonClicked" })
130+
})
131+
})
132+
133+
// Helper function to get user message content
134+
const getUserMessageContent = (clineInstance: Cline) => {
135+
const clineAny = clineInstance as any
136+
return clineAny.userMessageContent
137+
}
138+
139+
// Helper function to validate response lines
140+
const validateResponseLines = (
141+
responseLines: string[],
142+
options: {
143+
expectedLineCount: number
144+
shouldContainLines?: number[]
145+
shouldNotContainLines?: number[]
146+
},
147+
) => {
148+
if (options.shouldContainLines) {
149+
const contentLines = responseLines.filter((line) => line.includes("Line "))
150+
expect(contentLines.length).toBe(options.expectedLineCount)
151+
options.shouldContainLines.forEach((lineNum) => {
152+
expect(contentLines[lineNum - 1]).toContain(`Line ${lineNum}`)
153+
})
154+
}
155+
156+
if (options.shouldNotContainLines) {
157+
options.shouldNotContainLines.forEach((lineNum) => {
158+
expect(responseLines.some((line) => line.includes(`Line ${lineNum}`))).toBe(false)
159+
})
160+
}
161+
}
162+
163+
interface TestExpectations {
164+
extractTextCalled: boolean
165+
readLinesCalled: boolean
166+
sourceCodeDefCalled: boolean
167+
readLinesParams?: [string, number, number]
168+
responseValidation: {
169+
expectedLineCount: number
170+
shouldContainLines?: number[]
171+
shouldNotContainLines?: number[]
172+
}
173+
expectedContent?: string
174+
truncationMessage?: string
175+
includeSourceCodeDef?: boolean
176+
}
177+
178+
interface TestCase {
179+
name: string
180+
maxReadFileLine: number
181+
setup?: () => void
182+
expectations: TestExpectations
183+
}
184+
185+
// Test cases
186+
const testCases: TestCase[] = [
187+
{
188+
name: "read entire file when maxReadFileLine is -1",
189+
maxReadFileLine: -1,
190+
expectations: {
191+
extractTextCalled: true,
192+
readLinesCalled: false,
193+
sourceCodeDefCalled: false,
194+
responseValidation: {
195+
expectedLineCount: 5,
196+
shouldContainLines: [1, 2, 3, 4, 5],
197+
},
198+
expectedContent: numberedFileContent,
199+
},
200+
},
201+
{
202+
name: "read entire file when maxReadFileLine >= file length",
203+
maxReadFileLine: 10,
204+
expectations: {
205+
extractTextCalled: true,
206+
readLinesCalled: false,
207+
sourceCodeDefCalled: false,
208+
responseValidation: {
209+
expectedLineCount: 5,
210+
shouldContainLines: [1, 2, 3, 4, 5],
211+
},
212+
expectedContent: numberedFileContent,
213+
},
214+
},
215+
{
216+
name: "read zero lines and only provide line declaration definitions when maxReadFileLine is 0",
217+
maxReadFileLine: 0,
218+
expectations: {
219+
extractTextCalled: false,
220+
readLinesCalled: false,
221+
sourceCodeDefCalled: true,
222+
responseValidation: {
223+
expectedLineCount: 0,
224+
},
225+
truncationMessage: `[Showing only 0 of 5 total lines. Use start_line and end_line if you need to read more]`,
226+
includeSourceCodeDef: true,
227+
},
228+
},
229+
{
230+
name: "read maxReadFileLine lines and provide line declaration definitions when maxReadFileLine < file length",
231+
maxReadFileLine: 3,
232+
setup: () => {
233+
jest.clearAllMocks()
234+
;(countFileLines as jest.Mock).mockResolvedValue(5)
235+
;(readLines as jest.Mock).mockImplementation((path, endLine, startLine = 0) => {
236+
const lines = fileContent.split("\n")
237+
const actualEndLine = endLine !== undefined ? Math.min(endLine, lines.length - 1) : lines.length - 1
238+
const actualStartLine = startLine !== undefined ? Math.min(startLine, lines.length - 1) : 0
239+
const requestedLines = lines.slice(actualStartLine, actualEndLine + 1)
240+
return Promise.resolve(requestedLines.join("\n"))
241+
})
242+
},
243+
expectations: {
244+
extractTextCalled: false,
245+
readLinesCalled: true,
246+
sourceCodeDefCalled: true,
247+
readLinesParams: [absoluteFilePath, 2, 0],
248+
responseValidation: {
249+
expectedLineCount: 3,
250+
shouldContainLines: [1, 2, 3],
251+
shouldNotContainLines: [4, 5],
252+
},
253+
truncationMessage: `[Showing only 3 of 5 total lines. Use start_line and end_line if you need to read more]`,
254+
includeSourceCodeDef: true,
255+
},
256+
},
257+
]
258+
259+
test.each(testCases)("should $name", async (testCase) => {
260+
// Setup
261+
if (testCase.setup) {
262+
testCase.setup()
263+
}
264+
mockProvider.getState.mockResolvedValue({ maxReadFileLine: testCase.maxReadFileLine })
265+
266+
// Execute
267+
await cline.presentAssistantMessage()
268+
269+
// Verify mock calls
270+
if (testCase.expectations.extractTextCalled) {
271+
expect(extractTextFromFile).toHaveBeenCalledWith(absoluteFilePath)
272+
} else {
273+
expect(extractTextFromFile).not.toHaveBeenCalled()
274+
}
275+
276+
if (testCase.expectations.readLinesCalled) {
277+
const params = testCase.expectations.readLinesParams
278+
if (!params) {
279+
throw new Error("readLinesParams must be defined when readLinesCalled is true")
280+
}
281+
expect(readLines).toHaveBeenCalledWith(...params)
282+
} else {
283+
expect(readLines).not.toHaveBeenCalled()
284+
}
285+
286+
if (testCase.expectations.sourceCodeDefCalled) {
287+
expect(parseSourceCodeDefinitionsForFile).toHaveBeenCalled()
288+
} else {
289+
expect(parseSourceCodeDefinitionsForFile).not.toHaveBeenCalled()
290+
}
291+
292+
// Verify response content
293+
const userMessageContent = getUserMessageContent(cline)
294+
295+
if (DEBUG) {
296+
console.log(`\n=== Test: ${testCase.name} ===`)
297+
console.log(`maxReadFileLine: ${testCase.maxReadFileLine}`)
298+
console.log("Response content:", JSON.stringify(userMessageContent, null, 2))
299+
}
300+
const responseLines = userMessageContent[1].text.split("\n")
301+
302+
if (DEBUG) {
303+
console.log(`Number of lines in response: ${responseLines.length}`)
304+
}
305+
306+
expect(userMessageContent.length).toBe(2)
307+
expect(userMessageContent[0].text).toBe(`[read_file for '${testFilePath}'] Result:`)
308+
309+
if (testCase.expectations.expectedContent) {
310+
expect(userMessageContent[1].text).toBe(testCase.expectations.expectedContent)
311+
}
312+
313+
if (testCase.expectations.responseValidation) {
314+
validateResponseLines(responseLines, testCase.expectations.responseValidation)
315+
}
316+
317+
if (testCase.expectations.truncationMessage) {
318+
expect(userMessageContent[1].text).toContain(testCase.expectations.truncationMessage)
319+
}
320+
321+
if (testCase.expectations.includeSourceCodeDef) {
322+
expect(userMessageContent[1].text).toContain(sourceCodeDef)
323+
}
324+
})
325+
})

0 commit comments

Comments
 (0)