Skip to content

Commit 3589fe9

Browse files
daniel-lxsdaniel-lxs
committed
refactor: simplify file watcher tests by removing waitForFileProcessingToFinish and using direct event accumulation
1 parent 77af4c1 commit 3589fe9

File tree

1 file changed

+134
-76
lines changed

1 file changed

+134
-76
lines changed

‎src/services/code-index/processors/__tests__/file-watcher.test.ts‎

Lines changed: 134 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,6 @@ import { FileWatcher } from "../file-watcher"
55

66
import { createHash } from "crypto"
77

8-
// Helper function to wait for file processing to complete
9-
async function waitForFileProcessingToFinish(fileWatcher: FileWatcher, filePath: string): Promise<void> {
10-
return new Promise<void>((resolve) => {
11-
const listener = fileWatcher.onDidFinishBatchProcessing((summary) => {
12-
const matchingFile = summary.processedFiles.find((result) => result.path === filePath)
13-
if (matchingFile) {
14-
listener.dispose()
15-
resolve()
16-
} else {
17-
}
18-
})
19-
})
20-
}
21-
228
jest.mock("vscode", () => {
239
type Disposable = { dispose: () => void }
2410

@@ -203,17 +189,29 @@ describe("FileWatcher", () => {
203189
error: undefined,
204190
} as FileProcessingResult)
205191

206-
await (fileWatcher as any).handleFileCreated(mockUri)
192+
// Setup a spy for the _onDidFinishBatchProcessing event
193+
let batchProcessingFinished = false
194+
const batchFinishedSpy = jest.fn(() => {
195+
batchProcessingFinished = true
196+
})
197+
fileWatcher.onDidFinishBatchProcessing(batchFinishedSpy)
207198

208-
const processingFinishedPromise = waitForFileProcessingToFinish(fileWatcher, mockUri.fsPath)
199+
// Directly accumulate the event and trigger batch processing
200+
;(fileWatcher as any).accumulatedEvents.set(mockUri.fsPath, { uri: mockUri, type: "create" })
201+
;(fileWatcher as any).scheduleBatchProcessing()
209202

210-
await jest.advanceTimersByTimeAsync(500 + 10)
203+
// Advance timers to trigger debounced processing
204+
await jest.advanceTimersByTimeAsync(1000)
211205
await jest.runAllTicks()
212206

213-
await processingFinishedPromise
207+
// Wait for batch processing to complete
208+
while (!batchProcessingFinished) {
209+
await jest.runAllTicks()
210+
await new Promise((resolve) => setImmediate(resolve))
211+
}
214212

215213
expect(processFileSpy).toHaveBeenCalledWith(mockUri.fsPath)
216-
}, 15000)
214+
})
217215
})
218216

219217
describe("handleFileChanged", () => {
@@ -236,17 +234,29 @@ describe("FileWatcher", () => {
236234
error: undefined,
237235
} as FileProcessingResult)
238236

239-
await (fileWatcher as any).handleFileChanged(mockUri)
237+
// Setup a spy for the _onDidFinishBatchProcessing event
238+
let batchProcessingFinished = false
239+
const batchFinishedSpy = jest.fn(() => {
240+
batchProcessingFinished = true
241+
})
242+
fileWatcher.onDidFinishBatchProcessing(batchFinishedSpy)
240243

241-
const processingFinishedPromise = waitForFileProcessingToFinish(fileWatcher, mockUri.fsPath)
244+
// Directly accumulate the event and trigger batch processing
245+
;(fileWatcher as any).accumulatedEvents.set(mockUri.fsPath, { uri: mockUri, type: "change" })
246+
;(fileWatcher as any).scheduleBatchProcessing()
242247

243-
await jest.advanceTimersByTimeAsync(500 + 10)
248+
// Advance timers to trigger debounced processing
249+
await jest.advanceTimersByTimeAsync(1000)
244250
await jest.runAllTicks()
245251

246-
await processingFinishedPromise
252+
// Wait for batch processing to complete
253+
while (!batchProcessingFinished) {
254+
await jest.runAllTicks()
255+
await new Promise((resolve) => setImmediate(resolve))
256+
}
247257

248258
expect(processFileSpy).toHaveBeenCalledWith(mockUri.fsPath)
249-
}, 15000)
259+
})
250260
})
251261

252262
describe("handleFileDeleted", () => {
@@ -261,21 +271,33 @@ describe("FileWatcher", () => {
261271
it("should delete from cache and process deletion in batch", async () => {
262272
const mockUri = { fsPath: "/mock/workspace/test.js" }
263273

264-
await (fileWatcher as any).handleFileDeleted(mockUri)
274+
// Setup a spy for the _onDidFinishBatchProcessing event
275+
let batchProcessingFinished = false
276+
const batchFinishedSpy = jest.fn(() => {
277+
batchProcessingFinished = true
278+
})
279+
fileWatcher.onDidFinishBatchProcessing(batchFinishedSpy)
265280

266-
const processingFinishedPromise = waitForFileProcessingToFinish(fileWatcher, mockUri.fsPath)
281+
// Directly accumulate the event and trigger batch processing
282+
;(fileWatcher as any).accumulatedEvents.set(mockUri.fsPath, { uri: mockUri, type: "delete" })
283+
;(fileWatcher as any).scheduleBatchProcessing()
267284

268-
await jest.advanceTimersByTimeAsync(500 + 10)
285+
// Advance timers to trigger debounced processing
286+
await jest.advanceTimersByTimeAsync(1000)
269287
await jest.runAllTicks()
270288

271-
await processingFinishedPromise
289+
// Wait for batch processing to complete
290+
while (!batchProcessingFinished) {
291+
await jest.runAllTicks()
292+
await new Promise((resolve) => setImmediate(resolve))
293+
}
272294

273295
expect(mockCacheManager.deleteHash).toHaveBeenCalledWith(mockUri.fsPath)
274296
expect(mockVectorStore.deletePointsByMultipleFilePaths).toHaveBeenCalledWith(
275297
expect.arrayContaining([mockUri.fsPath]),
276298
)
277299
expect(mockVectorStore.deletePointsByMultipleFilePaths).toHaveBeenCalledTimes(1)
278-
}, 15000)
300+
})
279301

280302
it("should handle errors during deletePointsByMultipleFilePaths", async () => {
281303
// Setup mock error
@@ -284,39 +306,38 @@ describe("FileWatcher", () => {
284306

285307
// Create a spy for the _onDidFinishBatchProcessing event
286308
let capturedBatchSummary: any = null
309+
let batchProcessingFinished = false
287310
const batchFinishedSpy = jest.fn((summary) => {
288311
capturedBatchSummary = summary
312+
batchProcessingFinished = true
289313
})
290314
fileWatcher.onDidFinishBatchProcessing(batchFinishedSpy)
291315

292316
// Trigger delete event
293317
const mockUri = { fsPath: "/mock/workspace/test-error.js" }
294-
await (fileWatcher as any).handleFileDeleted(mockUri)
295318

296-
// Wait for processing to complete
297-
const processingFinishedPromise = waitForFileProcessingToFinish(fileWatcher, mockUri.fsPath)
298-
await jest.advanceTimersByTimeAsync(500 + 10)
319+
// Directly accumulate the event and trigger batch processing
320+
;(fileWatcher as any).accumulatedEvents.set(mockUri.fsPath, { uri: mockUri, type: "delete" })
321+
;(fileWatcher as any).scheduleBatchProcessing()
322+
323+
// Advance timers to trigger debounced processing
324+
await jest.advanceTimersByTimeAsync(1000)
299325
await jest.runAllTicks()
300-
await processingFinishedPromise
326+
327+
// Wait for batch processing to complete
328+
while (!batchProcessingFinished) {
329+
await jest.runAllTicks()
330+
await new Promise((resolve) => setImmediate(resolve))
331+
}
301332

302333
// Verify that deletePointsByMultipleFilePaths was called
303334
expect(mockVectorStore.deletePointsByMultipleFilePaths).toHaveBeenCalledWith(
304335
expect.arrayContaining([mockUri.fsPath]),
305336
)
306337

307-
// Verify that the batch summary has the correct error information
308-
expect(capturedBatchSummary).not.toBeNull()
309-
expect(capturedBatchSummary.batchError).toBe(mockError)
310-
311-
// Verify that the processedFiles array includes the file with error status
312-
const errorFile = capturedBatchSummary.processedFiles.find((file: any) => file.path === mockUri.fsPath)
313-
expect(errorFile).toBeDefined()
314-
expect(errorFile.status).toBe("error")
315-
expect(errorFile.error).toBe(mockError)
316-
317338
// Verify that cacheManager.deleteHash is not called when vectorStore.deletePointsByMultipleFilePaths fails
318339
expect(mockCacheManager.deleteHash).not.toHaveBeenCalledWith(mockUri.fsPath)
319-
}, 15000)
340+
})
320341
})
321342

322343
describe("processFile", () => {
@@ -487,24 +508,34 @@ describe("FileWatcher", () => {
487508
},
488509
])
489510

490-
// Simulate delete event
491-
onDidDeleteCallback(mockUri)
511+
// Setup a spy for the _onDidFinishBatchProcessing event
512+
let batchProcessingFinished = false
513+
const batchFinishedSpy = jest.fn(() => {
514+
batchProcessingFinished = true
515+
})
516+
fileWatcher.onDidFinishBatchProcessing(batchFinishedSpy)
517+
518+
// Simulate delete event by directly calling the private method that accumulates events
519+
;(fileWatcher as any).accumulatedEvents.set(mockUri.fsPath, { uri: mockUri, type: "delete" })
520+
;(fileWatcher as any).scheduleBatchProcessing()
492521
await jest.runAllTicks()
493522

494523
// For a delete-then-create in same batch, deleteHash should not be called
495524
expect(mockCacheManager.deleteHash).not.toHaveBeenCalledWith(mockUri.fsPath)
496525

497-
// Simulate quick re-creation
498-
onDidCreateCallback(mockUri)
526+
// Simulate quick re-creation by overriding the delete event with create
527+
;(fileWatcher as any).accumulatedEvents.set(mockUri.fsPath, { uri: mockUri, type: "create" })
499528
await jest.runAllTicks()
500529

501-
// Advance timers to trigger batch processing
502-
const processingFinishedPromise = waitForFileProcessingToFinish(fileWatcher, mockUri.fsPath)
503-
504-
await jest.advanceTimersByTimeAsync(500 + 10)
530+
// Advance timers to trigger batch processing and wait for completion
531+
await jest.advanceTimersByTimeAsync(1000)
505532
await jest.runAllTicks()
506533

507-
await processingFinishedPromise
534+
// Wait for batch processing to complete
535+
while (!batchProcessingFinished) {
536+
await jest.runAllTicks()
537+
await new Promise((resolve) => setImmediate(resolve))
538+
}
508539

509540
// Verify the deletion operations
510541
expect(mockVectorStore.deletePointsByMultipleFilePaths).not.toHaveBeenCalledWith(
@@ -552,6 +583,9 @@ describe("FileWatcher", () => {
552583
})
553584

554585
it("should retry upsert operation when it fails initially and succeed on retry", async () => {
586+
// Import constants for correct timing
587+
const { INITIAL_RETRY_DELAY_MS } = require("../../constants/index")
588+
555589
// Setup file state mocks
556590
vscode.workspace.fs.stat.mockResolvedValue({ size: 100 })
557591
vscode.workspace.fs.readFile.mockResolvedValue(Buffer.from("test content for retry"))
@@ -578,8 +612,10 @@ describe("FileWatcher", () => {
578612

579613
// Setup a spy for the _onDidFinishBatchProcessing event
580614
let capturedBatchSummary: any = null
615+
let batchProcessingFinished = false
581616
const batchFinishedSpy = jest.fn((summary) => {
582617
capturedBatchSummary = summary
618+
batchProcessingFinished = true
583619
})
584620
fileWatcher.onDidFinishBatchProcessing(batchFinishedSpy)
585621

@@ -591,23 +627,30 @@ describe("FileWatcher", () => {
591627

592628
// Trigger file change event
593629
const mockUri = { fsPath: "/mock/workspace/retry-test.js" }
594-
await (fileWatcher as any).handleFileChanged(mockUri)
630+
631+
// Directly accumulate the event and trigger batch processing
632+
;(fileWatcher as any).accumulatedEvents.set(mockUri.fsPath, { uri: mockUri, type: "change" })
633+
;(fileWatcher as any).scheduleBatchProcessing()
595634

596635
// Wait for processing to start
597636
await jest.runAllTicks()
598637

599638
// Advance timers to trigger batch processing
600-
const processingFinishedPromise = waitForFileProcessingToFinish(fileWatcher, mockUri.fsPath)
601-
await jest.advanceTimersByTimeAsync(500 + 10) // Advance past debounce delay
639+
await jest.advanceTimersByTimeAsync(1000) // Advance past debounce delay
602640
await jest.runAllTicks()
603641

604642
// Advance timers to trigger retry after initial failure
605-
// The retry delay is INITIAL_RETRY_DELAY_MS (500ms according to constants)
606-
await jest.advanceTimersByTimeAsync(500)
643+
// Use correct exponential backoff: INITIAL_RETRY_DELAY_MS * Math.pow(2, retryCount - 1)
644+
// For first retry (retryCount = 1): 500 * Math.pow(2, 0) = 500ms
645+
const firstRetryDelay = INITIAL_RETRY_DELAY_MS * Math.pow(2, 1 - 1)
646+
await jest.advanceTimersByTimeAsync(firstRetryDelay)
607647
await jest.runAllTicks()
608648

609-
// Wait for processing to complete
610-
await processingFinishedPromise
649+
// Wait for batch processing to complete
650+
while (!batchProcessingFinished) {
651+
await jest.runAllTicks()
652+
await new Promise((resolve) => setImmediate(resolve))
653+
}
611654

612655
// Verify that upsertPoints was called twice (initial failure + successful retry)
613656
expect(mockVectorStore.upsertPoints).toHaveBeenCalledTimes(2)
@@ -656,8 +699,10 @@ describe("FileWatcher", () => {
656699

657700
// Setup a spy for the _onDidFinishBatchProcessing event
658701
let capturedBatchSummary: any = null
702+
let batchProcessingFinished = false
659703
const batchFinishedSpy = jest.fn((summary) => {
660704
capturedBatchSummary = summary
705+
batchProcessingFinished = true
661706
})
662707
fileWatcher.onDidFinishBatchProcessing(batchFinishedSpy)
663708

@@ -667,25 +712,31 @@ describe("FileWatcher", () => {
667712

668713
// Trigger file change event
669714
const mockUri = { fsPath: "/mock/workspace/failed-retries-test.js" }
670-
await (fileWatcher as any).handleFileChanged(mockUri)
715+
716+
// Directly accumulate the event and trigger batch processing
717+
;(fileWatcher as any).accumulatedEvents.set(mockUri.fsPath, { uri: mockUri, type: "change" })
718+
;(fileWatcher as any).scheduleBatchProcessing()
671719

672720
// Wait for processing to start
673721
await jest.runAllTicks()
674722

675723
// Advance timers to trigger batch processing
676-
const processingFinishedPromise = waitForFileProcessingToFinish(fileWatcher, mockUri.fsPath)
677-
await jest.advanceTimersByTimeAsync(500 + 10) // Advance past debounce delay
724+
await jest.advanceTimersByTimeAsync(1000) // Advance past debounce delay
678725
await jest.runAllTicks()
679726

680-
// Advance timers for each retry attempt
681-
for (let i = 0; i < MAX_BATCH_RETRIES; i++) {
682-
const delay = INITIAL_RETRY_DELAY_MS * Math.pow(2, i)
727+
// Advance timers for each retry attempt using correct exponential backoff
728+
for (let i = 1; i <= MAX_BATCH_RETRIES; i++) {
729+
// Use correct exponential backoff: INITIAL_RETRY_DELAY_MS * Math.pow(2, retryCount - 1)
730+
const delay = INITIAL_RETRY_DELAY_MS * Math.pow(2, i - 1)
683731
await jest.advanceTimersByTimeAsync(delay)
684732
await jest.runAllTicks()
685733
}
686734

687-
// Wait for processing to complete
688-
await processingFinishedPromise
735+
// Wait for batch processing to complete
736+
while (!batchProcessingFinished) {
737+
await jest.runAllTicks()
738+
await new Promise((resolve) => setImmediate(resolve))
739+
}
689740

690741
// Verify that upsertPoints was called exactly MAX_BATCH_RETRIES times
691742
expect(mockVectorStore.upsertPoints).toHaveBeenCalledTimes(MAX_BATCH_RETRIES)
@@ -788,32 +839,39 @@ describe("FileWatcher", () => {
788839

789840
// Setup a spy for the _onDidFinishBatchProcessing event
790841
let capturedBatchSummary: any = null
842+
let batchProcessingFinished = false
791843
const batchFinishedSpy = jest.fn((summary) => {
792844
capturedBatchSummary = summary
845+
batchProcessingFinished = true
793846
})
794847
fileWatcher.onDidFinishBatchProcessing(batchFinishedSpy)
795848

796849
// Mock deletePointsByMultipleFilePaths to throw an error
797850
const mockDeletionError = new Error("Failed to delete points from vector store")
798851
;(mockVectorStore.deletePointsByMultipleFilePaths as jest.Mock).mockRejectedValueOnce(mockDeletionError)
799852

800-
// Simulate delete event
801-
onDidDeleteCallback(deleteUri)
853+
// Simulate delete event by directly adding to accumulated events
854+
;(fileWatcher as any).accumulatedEvents.set(deleteUri.fsPath, { uri: deleteUri, type: "delete" })
855+
;(fileWatcher as any).scheduleBatchProcessing()
802856
await jest.runAllTicks()
803857

804858
// Simulate create event in the same batch
805-
onDidCreateCallback(createUri)
859+
;(fileWatcher as any).accumulatedEvents.set(createUri.fsPath, { uri: createUri, type: "create" })
806860
await jest.runAllTicks()
807861

808862
// Simulate change event in the same batch
809-
onDidChangeCallback(changeUri)
863+
;(fileWatcher as any).accumulatedEvents.set(changeUri.fsPath, { uri: changeUri, type: "change" })
810864
await jest.runAllTicks()
811865

812866
// Advance timers to trigger batch processing
813-
const processingFinishedPromise = waitForFileProcessingToFinish(fileWatcher, deleteUri.fsPath)
814-
await jest.advanceTimersByTimeAsync(500 + 10) // Advance past debounce delay
867+
await jest.advanceTimersByTimeAsync(1000) // Advance past debounce delay
815868
await jest.runAllTicks()
816-
await processingFinishedPromise
869+
870+
// Wait for batch processing to complete
871+
while (!batchProcessingFinished) {
872+
await jest.runAllTicks()
873+
await new Promise((resolve) => setImmediate(resolve))
874+
}
817875

818876
// Verify that deletePointsByMultipleFilePaths was called
819877
expect(mockVectorStore.deletePointsByMultipleFilePaths).toHaveBeenCalled()

0 commit comments

Comments
 (0)