From 7b1ee18ff85c111f70922a3ca07ba2a7bd1038b8 Mon Sep 17 00:00:00 2001 From: Christian Vuerings Date: Sun, 13 Apr 2025 17:00:06 -0700 Subject: [PATCH] fix: implement just-in-time test file list updates This change optimizes file system event handling by implementing a just-in-time approach to updating test file lists. Rather than immediately refreshing the test file list on every file system event, we now mark it as dirty and only update it when necessary (before running tests). - Add testFileListDirty flag to track when updates are needed - Only update test file list when running or debugging tests - Force update on initial startup for completeness - Significantly improves performance when renaming files in large repositories fixes #1196 --- src/JestExt/core.ts | 27 ++++++-- src/TestResults/TestResultProvider.ts | 10 +++ tests/JestExt/core.test.ts | 72 ++++++++++++++++---- tests/TestResults/TestResultProvider.test.ts | 9 +++ 4 files changed, 101 insertions(+), 17 deletions(-) diff --git a/src/JestExt/core.ts b/src/JestExt/core.ts index 1ddf6070a..fe8638481 100644 --- a/src/JestExt/core.ts +++ b/src/JestExt/core.ts @@ -323,7 +323,8 @@ export class JestExt { this.events.onTestSessionStarted.fire({ ...this.extContext, session: this.processSession }); - await this.updateTestFileList(); + // Force update test file list during session initialization + await this.updateTestFileList(true); // update visible editors that belong to this folder this.updateVisibleTextEditors(); @@ -568,6 +569,11 @@ export class JestExt { //** commands */ public debugTests = async (debugInfo: DebugInfo): Promise => { + // Check if test file list needs updating before debug, and update if necessary + if (this.testResultProvider.isTestFileListDirty()) { + await this.updateTestFileList(); + } + const getDebugConfig = ( folder?: vscode.WorkspaceFolder ): vscode.DebugConfiguration | undefined => { @@ -619,6 +625,11 @@ export class JestExt { this.enableOutputOnRun(); await this.exitDeferMode(); + // Check if test file list needs updating, and update if necessary + if (this.testResultProvider.isTestFileListDirty()) { + await this.updateTestFileList(); + } + if (!editor) { if (this.processSession.scheduleProcess({ type: 'all-tests', nonBlocking: true })) { this.dirtyFiles.clear(); @@ -703,6 +714,7 @@ export class JestExt { private refreshDocumentChange(document?: vscode.TextDocument): void { this.updateVisibleTextEditors(document); + // Update status bar with latest stats this.updateStatusBar({ stats: this.toSBStats(this.testResultProvider.getTestSuiteStats()), }); @@ -741,7 +753,12 @@ export class JestExt { this.refreshDocumentChange(document); } - private async updateTestFileList(): Promise { + private async updateTestFileList(force: boolean = false): Promise { + // Skip update if not forced and test file list isn't dirty + if (!force && !this.testResultProvider.isTestFileListDirty()) { + return; + } + return new Promise((resolve, reject) => { this.processSession.scheduleProcess({ type: 'list-test-files', @@ -765,13 +782,13 @@ export class JestExt { } onDidCreateFiles(_event: vscode.FileCreateEvent): void { - this.updateTestFileList(); + this.testResultProvider.markTestFileListDirty(); } onDidRenameFiles(_event: vscode.FileRenameEvent): void { - this.updateTestFileList(); + this.testResultProvider.markTestFileListDirty(); } onDidDeleteFiles(_event: vscode.FileDeleteEvent): void { - this.updateTestFileList(); + this.testResultProvider.markTestFileListDirty(); } toggleCoverage(): Promise { diff --git a/src/TestResults/TestResultProvider.ts b/src/TestResults/TestResultProvider.ts index 198487ff8..0a64a9492 100644 --- a/src/TestResults/TestResultProvider.ts +++ b/src/TestResults/TestResultProvider.ts @@ -194,6 +194,7 @@ export class TestResultProvider { private testFiles?: string[]; private snapshotProvider: SnapshotProvider; private parser: Parser; + private testFileListDirty: boolean = false; constructor( extEvents: JestSessionEvents, @@ -258,12 +259,21 @@ export class TestResultProvider { updateTestFileList(testFiles?: string[]): void { this.testFiles = testFiles; + this.testFileListDirty = false; // clear the cache in case we have cached some non-test files prior this.testSuites.clear(); this.events.testListUpdated.fire(testFiles); } + + markTestFileListDirty(): void { + this.testFileListDirty = true; + } + + isTestFileListDirty(): boolean { + return this.testFileListDirty; + } getTestList(): string[] { if (this.testFiles && this.testFiles.length > 0) { return this.testFiles; diff --git a/tests/JestExt/core.test.ts b/tests/JestExt/core.test.ts index 43f808cca..fdce91c79 100644 --- a/tests/JestExt/core.test.ts +++ b/tests/JestExt/core.test.ts @@ -112,12 +112,20 @@ describe('JestExt', () => { const coverageCodeLensProvider: any = override?.coverageCodeLensProvider ?? { coverageChanged: jest.fn(), }; - return new JestExt( + + // Make a JestExt instance + const jestExtInstance = new JestExt( context, workspaceFolder, debugConfigurationProvider, coverageCodeLensProvider ); + + // Mock the new methods on testResultProvider + jestExtInstance.testResultProvider.markTestFileListDirty = jest.fn(); + jestExtInstance.testResultProvider.isTestFileListDirty = jest.fn().mockReturnValue(false); + + return jestExtInstance; }; const mockEditor = (fileName: string, languageId = 'typescript'): any => { return { @@ -178,6 +186,22 @@ describe('JestExt', () => { let startDebugging; const mockShowQuickPick = jest.fn(); let mockConfigurations = []; + + it('should update test file list if marked as dirty before debugging', async () => { + const sut = newJestExt(); + // Mock that test file list is dirty + (sut.testResultProvider.isTestFileListDirty as jest.Mock).mockReturnValueOnce(true); + + // Set up updateTestFileList to resolve immediately when called + const updateTestFileListSpy = jest.spyOn(sut as any, 'updateTestFileList').mockResolvedValueOnce(undefined); + + await sut.debugTests({ testPath: document.fileName, testName: 'testName' }); + + // Verify updateTestFileList was called before debugging + expect(updateTestFileListSpy).toHaveBeenCalled(); + expect(sut.debugConfigurationProvider.prepareTestRun).toHaveBeenCalled(); + }); + beforeEach(() => { startDebugging = vscode.debug.startDebugging as unknown as jest.Mock<{}>; (startDebugging as unknown as jest.Mock<{}>).mockImplementation( @@ -752,6 +776,19 @@ describe('JestExt', () => { expect(mockTestProvider.dispose).toHaveBeenCalledTimes(1); expect(JestTestProvider).toHaveBeenCalledTimes(2); }); + + it('forces update of test file list on session start', async () => { + const sut = createJestExt(); + + // Set up spy to check how updateTestFileList is called + const updateTestFileListSpy = jest.spyOn(sut as any, 'updateTestFileList'); + + await sut.startSession(); + + // Verify updateTestFileList was called with force=true + expect(updateTestFileListSpy).toHaveBeenCalledWith(true); + }); + describe('will update test file list', () => { it.each` fileNames | error | expectedTestFiles @@ -861,6 +898,21 @@ describe('JestExt', () => { }); }); describe('runAllTests', () => { + it('should update test file list if marked as dirty before running tests', async () => { + const sut = newJestExt(); + // Mock that test file list is dirty + (sut.testResultProvider.isTestFileListDirty as jest.Mock).mockReturnValueOnce(true); + + // Set up updateTestFileList to resolve immediately when called + const updateTestFileListSpy = jest.spyOn(sut as any, 'updateTestFileList').mockResolvedValueOnce(undefined); + + await sut.runAllTests(); + + // Verify updateTestFileList was called before scheduling process + expect(updateTestFileListSpy).toHaveBeenCalled(); + expect(mockProcessSession.scheduleProcess).toHaveBeenCalled(); + }); + describe.each` scheduleProcess ${{}} @@ -934,29 +986,25 @@ describe('JestExt', () => { } ); }); - describe('refresh test file list upon file system change', () => { - const getProcessType = () => { - const { type } = mockProcessSession.scheduleProcess.mock.calls[0][0]; - return type; - }; + describe('mark test file list as dirty upon file system change', () => { let jestExt: any; beforeEach(() => { jestExt = newJestExt(); }); it('when new file is created', () => { jestExt.onDidCreateFiles({}); - expect(mockProcessSession.scheduleProcess).toHaveBeenCalledTimes(1); - expect(getProcessType()).toEqual('list-test-files'); + expect(jestExt.testResultProvider.markTestFileListDirty).toHaveBeenCalled(); + expect(mockProcessSession.scheduleProcess).not.toHaveBeenCalled(); }); it('when file is renamed', () => { jestExt.onDidRenameFiles({}); - expect(mockProcessSession.scheduleProcess).toHaveBeenCalledTimes(1); - expect(getProcessType()).toEqual('list-test-files'); + expect(jestExt.testResultProvider.markTestFileListDirty).toHaveBeenCalled(); + expect(mockProcessSession.scheduleProcess).not.toHaveBeenCalled(); }); it('when file is deleted', () => { jestExt.onDidDeleteFiles({}); - expect(mockProcessSession.scheduleProcess).toHaveBeenCalledTimes(1); - expect(getProcessType()).toEqual('list-test-files'); + expect(jestExt.testResultProvider.markTestFileListDirty).toHaveBeenCalled(); + expect(mockProcessSession.scheduleProcess).not.toHaveBeenCalled(); }); }); describe('triggerUpdateSettings', () => { diff --git a/tests/TestResults/TestResultProvider.test.ts b/tests/TestResults/TestResultProvider.test.ts index c17fd765a..d0710122e 100644 --- a/tests/TestResults/TestResultProvider.test.ts +++ b/tests/TestResults/TestResultProvider.test.ts @@ -570,6 +570,15 @@ describe('TestResultProvider', () => { sut.updateTestFileList(['test-file']); itBlocks = []; }; + + it('should mark test file list as not dirty after update', () => { + const sut = new TestResultProvider(eventsMock); + expect(sut.isTestFileListDirty()).toBeFalsy(); + sut.markTestFileListDirty(); + expect(sut.isTestFileListDirty()).toBeTruthy(); + sut.updateTestFileList(['test-file']); + expect(sut.isTestFileListDirty()).toBeFalsy(); + }); it.each` desc | setup | itBlockOverride | expectedResults | statsChange ${'parse failed'} | ${forceParseError} | ${undefined} | ${[]} | ${'fail'}