diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index d221f5a8..54527667 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -78,17 +78,19 @@ async function getCommonPackages(): Promise { } async function selectWorkspaceOrCommon( - installable: Installable[], + api: PythonEnvironmentApi, + projects: PythonProject[] | undefined, + hasWorkspaceDeps: boolean, common: Installable[], showSkipOption: boolean, installed: string[], ): Promise { - if (installable.length === 0 && common.length === 0) { + if (!hasWorkspaceDeps && common.length === 0) { return undefined; } const items: QuickPickItem[] = []; - if (installable.length > 0) { + if (hasWorkspaceDeps) { items.push({ label: PackageManagement.workspaceDependencies, description: PackageManagement.workspaceDependenciesDescription, @@ -124,6 +126,12 @@ async function selectWorkspaceOrCommon( if (selected && !Array.isArray(selected)) { try { if (selected.label === PackageManagement.workspaceDependencies) { + // Lazy load: only search for dependencies when user selects this option + const installable = await getProjectInstallable(api, projects); + if (installable.length === 0) { + // No dependencies found, return to picker + return selectWorkspaceOrCommon(api, projects, hasWorkspaceDeps, common, showSkipOption, installed); + } return await selectFromInstallableToInstall(installable, undefined, { showBackButton }); } else if (selected.label === PackageManagement.searchCommonPackages) { return await selectFromCommonPackagesToInstall(common, installed, undefined, { showBackButton }); @@ -136,7 +144,7 @@ async function selectWorkspaceOrCommon( // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (ex: any) { if (ex === QuickInputButtons.Back) { - return selectWorkspaceOrCommon(installable, common, showSkipOption, installed); + return selectWorkspaceOrCommon(api, projects, hasWorkspaceDeps, common, showSkipOption, installed); } } } @@ -155,14 +163,36 @@ export async function getWorkspacePackagesToInstall( environment?: PythonEnvironment, log?: LogOutputChannel, ): Promise { - const installable = (await getProjectInstallable(api, project)) ?? []; + // Quick check if any dependency files exist (don't load/parse them yet) + const hasWorkspaceDeps = await hasProjectDependencies(project); let common = await getCommonPackages(); let installed: string[] | undefined; if (environment) { installed = (await refreshPipPackages(environment, log, { showProgress: true }))?.map((pkg) => pkg.name); common = mergePackages(common, installed ?? []); } - return selectWorkspaceOrCommon(installable, common, !!options.showSkipOption, installed ?? []); + return selectWorkspaceOrCommon(api, project, hasWorkspaceDeps, common, !!options.showSkipOption, installed ?? []); +} + +/** + * Quickly check if any project dependency files exist. + * This is a fast check that doesn't parse files, used to determine whether to show the option. + */ +export async function hasProjectDependencies(projects?: PythonProject[]): Promise { + if (!projects || projects.length === 0) { + return false; + } + const exclude = '**/{.venv*,.git,.nox,.tox,.conda,site-packages,__pypackages__}/**'; + + // Quick check: just see if ANY files exist (maxResults=1 for performance) + const results = await Promise.all([ + findFiles('**/*requirements*.txt', exclude, 1), + findFiles('*requirements*.txt', exclude, 1), + findFiles('**/requirements/*.txt', exclude, 1), + findFiles('**/pyproject.toml', exclude, 1), + ]); + + return results.some((uris) => uris.length > 0); } export async function getProjectInstallable( @@ -178,6 +208,7 @@ export async function getProjectInstallable( { location: ProgressLocation.Notification, title: VenvManagerStrings.searchingDependencies, + cancellable: true, }, async (_progress, token) => { const results: Uri[] = ( diff --git a/src/managers/builtin/venvStepBasedFlow.ts b/src/managers/builtin/venvStepBasedFlow.ts index b679cd62..97f96129 100644 --- a/src/managers/builtin/venvStepBasedFlow.ts +++ b/src/managers/builtin/venvStepBasedFlow.ts @@ -203,6 +203,8 @@ async function enterEnvironmentName(state: VenvCreationState): Promise { let findFilesStub: sinon.SinonStub; diff --git a/src/test/managers/builtin/pipUtils.unit.test.ts b/src/test/managers/builtin/pipUtils.unit.test.ts index 076596e8..49f8db0c 100644 --- a/src/test/managers/builtin/pipUtils.unit.test.ts +++ b/src/test/managers/builtin/pipUtils.unit.test.ts @@ -5,7 +5,7 @@ import { CancellationToken, Progress, ProgressOptions, Uri } from 'vscode'; import { PythonEnvironmentApi, PythonProject } from '../../../api'; import * as winapi from '../../../common/window.apis'; import * as wapi from '../../../common/workspace.apis'; -import { getProjectInstallable } from '../../../managers/builtin/pipUtils'; +import { getProjectInstallable, hasProjectDependencies } from '../../../managers/builtin/pipUtils'; suite('Pip Utils - getProjectInstallable', () => { let findFilesStub: sinon.SinonStub; @@ -199,4 +199,183 @@ suite('Pip Utils - getProjectInstallable', () => { assert.ok(firstResult.uri, 'Should have a URI'); assert.ok(firstResult.uri.fsPath.startsWith(workspacePath), 'Should be in workspace directory'); }); + + test('should show cancellable progress notification', async () => { + // Arrange: Mock findFiles to return empty results + findFilesStub.resolves([]); + + // Act: Call getProjectInstallable + const workspacePath = Uri.file('/test/path/root').fsPath; + const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }]; + await getProjectInstallable(mockApi as PythonEnvironmentApi, projects); + + // Assert: Verify withProgress was called with cancellable option + assert.ok(withProgressStub.calledOnce, 'Should call withProgress once'); + const progressOptions = withProgressStub.firstCall.args[0] as ProgressOptions; + assert.strictEqual(progressOptions.cancellable, true, 'Progress should be cancellable'); + }); + + test('should handle cancellation during file search', async () => { + // ARRANGE: Simulate a scenario where the user cancels the operation + // Step 1: Create a pre-cancelled token to simulate user clicking "Cancel" button + const cancelledToken: CancellationToken = { + isCancellationRequested: true, + onCancellationRequested: () => ({ dispose: () => {} }), + }; + + // Step 2: Override withProgress stub to pass the cancelled token to the search callback + // This simulates the progress dialog being cancelled and the token being propagated + withProgressStub.callsFake( + async ( + _options: ProgressOptions, + callback: ( + progress: Progress<{ message?: string; increment?: number }>, + token: CancellationToken, + ) => Thenable, + ) => { + // Execute the callback with the cancelled token (simulating cancellation during the operation) + return await callback({} as Progress<{ message?: string; increment?: number }>, cancelledToken); + }, + ); + + // Step 3: Mock findFiles to verify the cancelled token is properly passed through + // This ensures cancellation propagates from withProgress -> getProjectInstallable -> findFiles + findFilesStub.callsFake((_pattern: string, _exclude: string, _maxResults: number, token: CancellationToken) => { + // VERIFY: The same cancellation token should be passed to each findFiles call + assert.strictEqual(token, cancelledToken, 'Cancellation token should be passed to findFiles'); + return Promise.resolve([]); + }); + + // ACT: Call the function under test + const workspacePath = Uri.file('/test/path/root').fsPath; + const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }]; + await getProjectInstallable(mockApi as PythonEnvironmentApi, projects); + + // ASSERT: Verify the cancellation token was passed to all file search operations + // Even though cancelled, the function should attempt all searches (they'll just return empty quickly) + assert.ok(findFilesStub.called, 'findFiles should be called'); + // getProjectInstallable searches for dependencies using 4 different file patterns + assert.strictEqual(findFilesStub.callCount, 4, 'Should call findFiles 4 times for different patterns'); + }); +}); + +suite('Pip Utils - hasProjectDependencies', () => { + let findFilesStub: sinon.SinonStub; + + setup(() => { + findFilesStub = sinon.stub(wapi, 'findFiles'); + }); + + teardown(() => { + sinon.restore(); + }); + + test('should return true when requirements.txt exists', async () => { + // Arrange: Mock findFiles to return a requirements file + findFilesStub.callsFake((pattern: string, _exclude: string, maxResults?: number) => { + // Verify maxResults=1 is used for performance (quick check) + assert.strictEqual(maxResults, 1, 'Should use maxResults=1 for quick check'); + + if (pattern === '*requirements*.txt') { + return Promise.resolve([Uri.file('/test/path/root/requirements.txt')]); + } + return Promise.resolve([]); + }); + + // Act + const projects = [{ name: 'workspace', uri: Uri.file('/test/path/root') }]; + const result = await hasProjectDependencies(projects); + + // Assert + assert.strictEqual(result, true, 'Should return true when requirements files exist'); + }); + + test('should return true when pyproject.toml exists', async () => { + // Arrange: Mock findFiles to return pyproject.toml + findFilesStub.callsFake((pattern: string, _exclude: string, maxResults?: number) => { + assert.strictEqual(maxResults, 1, 'Should use maxResults=1 for quick check'); + + if (pattern === '**/pyproject.toml') { + return Promise.resolve([Uri.file('/test/path/root/pyproject.toml')]); + } + return Promise.resolve([]); + }); + + // Act + const projects = [{ name: 'workspace', uri: Uri.file('/test/path/root') }]; + const result = await hasProjectDependencies(projects); + + // Assert + assert.strictEqual(result, true, 'Should return true when pyproject.toml exists'); + }); + + test('should return false when no dependency files exist', async () => { + // Arrange: Mock findFiles to return empty arrays + findFilesStub.resolves([]); + + // Act + const projects = [{ name: 'workspace', uri: Uri.file('/test/path/root') }]; + const result = await hasProjectDependencies(projects); + + // Assert + assert.strictEqual(result, false, 'Should return false when no dependency files exist'); + // Verify all 4 patterns were checked + assert.strictEqual(findFilesStub.callCount, 4, 'Should check all 4 file patterns'); + }); + + test('should return false when no projects provided', async () => { + // Act + const result = await hasProjectDependencies(undefined); + + // Assert + assert.strictEqual(result, false, 'Should return false when no projects provided'); + assert.ok(!findFilesStub.called, 'Should not call findFiles when no projects'); + }); + + test('should return false when empty projects array provided', async () => { + // Act + const result = await hasProjectDependencies([]); + + // Assert + assert.strictEqual(result, false, 'Should return false when empty projects array'); + assert.ok(!findFilesStub.called, 'Should not call findFiles when projects array is empty'); + }); + + test('should use maxResults=1 for all patterns for performance', async () => { + // Arrange: Track all maxResults values + const maxResultsUsed: (number | undefined)[] = []; + findFilesStub.callsFake((_pattern: string, _exclude: string, maxResults?: number) => { + maxResultsUsed.push(maxResults); + return Promise.resolve([]); + }); + + // Act + const projects = [{ name: 'workspace', uri: Uri.file('/test/path/root') }]; + await hasProjectDependencies(projects); + + // Assert: All calls should use maxResults=1 for performance + assert.strictEqual(maxResultsUsed.length, 4, 'Should make 4 findFiles calls'); + maxResultsUsed.forEach((value, index) => { + assert.strictEqual(value, 1, `Call ${index + 1} should use maxResults=1`); + }); + }); + + test('should short-circuit when first pattern finds a file', async () => { + // Arrange: First pattern returns a result + findFilesStub.callsFake((pattern: string) => { + if (pattern === '**/*requirements*.txt') { + return Promise.resolve([Uri.file('/test/path/root/dev-requirements.txt')]); + } + return Promise.resolve([]); + }); + + // Act + const projects = [{ name: 'workspace', uri: Uri.file('/test/path/root') }]; + const result = await hasProjectDependencies(projects); + + // Assert: Should still return true even though only first pattern matched + assert.strictEqual(result, true, 'Should return true when any pattern finds files'); + // Note: All 4 patterns are checked in parallel with Promise.all, so all 4 calls happen + assert.strictEqual(findFilesStub.callCount, 4, 'All patterns checked in parallel'); + }); });