Merge duplicate progress notifications during environment creation

src/api.ts

@@ -739,6 +739,13 @@ export type PackageManagementOptions =
            * Show option to skip package installation or uninstallation.
            */
           showSkipOption?: boolean;
+
+          /**
+           * Suppress showing a progress notification when managing packages.
+           * Used when package management is part of a larger operation with its own progress notification.
+           */
+          suppressProgress?: boolean;
+
           /**
            * The list of packages to install.
            */
@@ -759,6 +766,13 @@ export type PackageManagementOptions =
            * Show option to skip package installation or uninstallation.
            */
           showSkipOption?: boolean;
+
+          /**
+           * Suppress showing a progress notification when managing packages.
+           * Used when package management is part of a larger operation with its own progress notification.
+           */
+          suppressProgress?: boolean;
+
           /**
            * The list of packages to install.
            */

src/managers/builtin/pipManager.ts

@@ -1,9 +1,11 @@
 import {
     CancellationError,
+    CancellationToken,
     Event,
     EventEmitter,
     LogOutputChannel,
     MarkdownString,
+    Progress,
     ProgressLocation,
     ThemeIcon,
     window,
@@ -77,34 +79,45 @@ export class PipPackageManager implements PackageManager, Disposable {
             install: toInstall,
             uninstall: toUninstall,
         };
-        await window.withProgress(
-            {
-                location: ProgressLocation.Notification,
-                title: 'Installing packages',
-                cancellable: true,
-            },
-            async (_progress, token) => {
-                try {
-                    const before = this.packages.get(environment.envId.id) ?? [];
-                    const after = await managePackages(environment, manageOptions, this.api, this, token);
-                    const changes = getChanges(before, after);
-                    this.packages.set(environment.envId.id, after);
-                    this._onDidChangePackages.fire({ environment, manager: this, changes });
-                } catch (e) {
-                    if (e instanceof CancellationError) {
-                        throw e;
-                    }
-                    this.log.error('Error managing packages', e);
-                    setImmediate(async () => {
-                        const result = await window.showErrorMessage('Error managing packages', 'View Output');
-                        if (result === 'View Output') {
-                            this.log.show();
-                        }
-                    });
+
+        const executeManage = async (
+            _progress: Progress<{ message?: string; increment?: number }> | undefined,
+            token: CancellationToken | undefined,
+        ) => {
+            try {
+                const before = this.packages.get(environment.envId.id) ?? [];
+                const after = await managePackages(environment, manageOptions, this.api, this, token);
+                const changes = getChanges(before, after);
+                this.packages.set(environment.envId.id, after);
+                this._onDidChangePackages.fire({ environment, manager: this, changes });
+            } catch (e) {
+                if (e instanceof CancellationError) {
                     throw e;
                 }
-            },
-        );
+                this.log.error('Error managing packages', e);
+                setImmediate(async () => {
+                    const result = await window.showErrorMessage('Error managing packages', 'View Output');
+                    if (result === 'View Output') {
+                        this.log.show();
+                    }
+                });
+                throw e;
+            }
+        };
+
+        if (options.suppressProgress) {
+            // When suppressProgress is true, execute without showing a separate progress notification
+            await executeManage(undefined, undefined);
+        } else {
+            await window.withProgress(
+                {
+                    location: ProgressLocation.Notification,
+                    title: 'Installing packages',
+                    cancellable: true,
+                },
+                executeManage,
+            );
+        }
     }
 
     async refresh(environment: PythonEnvironment): Promise<void> {

src/managers/builtin/venvUtils.ts

@@ -287,7 +287,7 @@ export async function createWithProgress(
                 basePython.version,
             ),
         },
-        async () => {
+        async (progress) => {
             const result: CreateEnvironmentResult = {};
             try {
                 const useUv = await isUvInstalled(log);
@@ -319,10 +319,13 @@ export async function createWithProgress(
                 // install packages
                 if (packages && (packages.install.length > 0 || packages.uninstall.length > 0)) {
                     try {
+                        // Update progress message to indicate package installation
+                        progress.report({ message: l10n.t('Installing packages...') });
                         await api.managePackages(env, {
                             upgrade: false,
                             install: packages?.install,
                             uninstall: packages?.uninstall ?? [],
+                            suppressProgress: true,
                         });
                     } catch (e) {
                         // error occurred while installing packages

src/managers/conda/condaPackageManager.ts

@@ -1,10 +1,13 @@
 import {
     CancellationError,
+    CancellationToken,
+    CancellationTokenSource,
     Disposable,
     Event,
     EventEmitter,
     LogOutputChannel,
     MarkdownString,
+    Progress,
     ProgressLocation,
 } from 'vscode';
 import {
@@ -70,31 +73,48 @@ export class CondaPackageManager implements PackageManager, Disposable {
             install: toInstall,
             uninstall: toUninstall,
         };
-        await withProgress(
-            {
-                location: ProgressLocation.Notification,
-                title: CondaStrings.condaInstallingPackages,
-                cancellable: true,
-            },
-            async (_progress, token) => {
-                try {
-                    const before = this.packages.get(environment.envId.id) ?? [];
-                    const after = await managePackages(environment, manageOptions, this.api, this, token, this.log);
-                    const changes = getChanges(before, after);
-                    this.packages.set(environment.envId.id, after);
-                    this._onDidChangePackages.fire({ environment: environment, manager: this, changes });
-                } catch (e) {
-                    if (e instanceof CancellationError) {
-                        throw e;
-                    }
 
-                    this.log.error('Error installing packages', e);
-                    setImmediate(async () => {
-                        await showErrorMessageWithLogs(CondaStrings.condaInstallError, this.log);
-                    });
+        const executeManage = async (
+            _progress: Progress<{ message?: string; increment?: number }> | undefined,
+            token: CancellationToken,
+        ) => {
+            try {
+                const before = this.packages.get(environment.envId.id) ?? [];
+                const after = await managePackages(environment, manageOptions, this.api, this, token, this.log);
+                const changes = getChanges(before, after);
+                this.packages.set(environment.envId.id, after);
+                this._onDidChangePackages.fire({ environment: environment, manager: this, changes });
+            } catch (e) {
+                if (e instanceof CancellationError) {
+                    throw e;
                 }
-            },
-        );
+
+                this.log.error('Error installing packages', e);
+                setImmediate(async () => {
+                    await showErrorMessageWithLogs(CondaStrings.condaInstallError, this.log);
+                });
+            }
+        };
+
+        if (options.suppressProgress) {
+            // When suppressProgress is true, execute without showing a separate progress notification
+            // Create a cancellation token source since conda's managePackages requires one
+            const tokenSource = new CancellationTokenSource();
+            try {
+                await executeManage(undefined, tokenSource.token);
+            } finally {
+                tokenSource.dispose();
+            }
+        } else {
+            await withProgress(
+                {
+                    location: ProgressLocation.Notification,
+                    title: CondaStrings.condaInstallingPackages,
+                    cancellable: true,
+                },
+                executeManage,
+            );
+        }
     }
 
     async refresh(environment: PythonEnvironment): Promise<void> {

src/test/managers/builtin/venvProgressMerge.unit.test.ts

@@ -0,0 +1,171 @@
+import assert from 'assert';
+import * as sinon from 'sinon';
+import { CancellationToken, LogOutputChannel, Progress, ProgressLocation, Uri } from 'vscode';
+import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi } from '../../../api';
+import * as winapi from '../../../common/window.apis';
+import { createWithProgress } from '../../../managers/builtin/venvUtils';
+
+suite('Venv Progress Merge Tests', () => {
+    let mockWithProgress: sinon.SinonStub;
+    let mockManagePackages: sinon.SinonStub;
+    let mockNativeFinder: {
+        resolve: sinon.SinonStub;
+        refresh: sinon.SinonStub;
+        dispose: sinon.SinonStub;
+    };
+    // Minimal mock that only implements the methods we need for this test
+    // Using type assertion to satisfy TypeScript since we only need createPythonEnvironmentItem and managePackages
+    let mockApi: {
+        createPythonEnvironmentItem: sinon.SinonStub;
+        managePackages: sinon.SinonStub;
+    };
+    let mockLog: LogOutputChannel;
+    let mockManager: EnvironmentManager;
+    let mockBasePython: PythonEnvironment;
+    let progressReportStub: sinon.SinonStub;
+
+    setup(() => {
+        // Stub withProgress to capture the progress callback
+        progressReportStub = sinon.stub();
+        mockWithProgress = sinon.stub(winapi, 'withProgress');
+        mockWithProgress.callsFake(
+            async (_options: { location: ProgressLocation; title: string }, callback: Function) => {
+                const mockProgress: Progress<{ message?: string; increment?: number }> = {
+                    report: progressReportStub,
+                };
+                const mockToken: CancellationToken = {
+                    isCancellationRequested: false,
+                    onCancellationRequested: sinon.stub(),
+                };
+                return await callback(mockProgress, mockToken);
+            },
+        );
+
+        // Create minimal mocks
+        mockNativeFinder = {
+            resolve: sinon.stub().resolves({
+                executable: '/test/venv/bin/python',
+                version: '3.11.0',
+                prefix: '/test/venv',
+                kind: 'venv',
+            }),
+            refresh: sinon.stub().resolves([]),
+            dispose: sinon.stub(),
+        };
+        mockApi = {
+            createPythonEnvironmentItem: sinon.stub().returns({
+                envId: { id: 'new-env', managerId: 'test-manager' },
+                name: 'New Venv',
+                version: '3.11.0',
+                environmentPath: Uri.file('/test/venv'),
+            }),
+            managePackages: sinon.stub().resolves(),
+        };
+        mockLog = {} as LogOutputChannel;
+        mockManager = { log: mockLog } as EnvironmentManager;
+
+        // Mock base Python environment
+        mockBasePython = {
+            envId: { id: 'test-env', managerId: 'test-manager' },
+            name: 'Test Python',
+            version: '3.11.0',
+            environmentPath: Uri.file('/test/python'),
+            execInfo: {
+                run: { executable: '/test/python/bin/python' },
+            },
+        } as PythonEnvironment;
+
+        // Mock managePackages - this is the key method we're testing
+        mockManagePackages = mockApi.managePackages;
+    });
+
+    teardown(() => {
+        sinon.restore();
+    });
+
+    test('should update progress message when installing packages', async () => {
+        // Mock file system check
+        const fsapi = require('fs-extra');
+        sinon.stub(fsapi, 'pathExists').resolves(true);
+
+        // Mock the Python run function
+        const helpers = require('../../../managers/builtin/helpers');
+        sinon.stub(helpers, 'isUvInstalled').resolves(false);
+        sinon.stub(helpers, 'runPython').resolves();
+
+        await createWithProgress(
+            mockNativeFinder,
+            mockApi as unknown as PythonEnvironmentApi,
+            mockLog,
+            mockManager,
+            mockBasePython,
+            Uri.file('/test'),
+            '/test/venv',
+            { install: ['numpy', 'pandas'], uninstall: [] },
+        );
+
+        // Verify the progress.report was called with the installing packages message
+        assert(progressReportStub.called, 'progress.report should have been called');
+        const reportCalls = progressReportStub.getCalls();
+        const installingPackagesCall = reportCalls.find((call) =>
+            call.args[0]?.message?.includes('Installing packages'),
+        );
+        assert(installingPackagesCall, 'progress.report should have been called with "Installing packages" message');
+    });
+
+    test('should pass suppressProgress:true when calling managePackages', async () => {
+        // Mock file system check
+        const fsapi = require('fs-extra');
+        sinon.stub(fsapi, 'pathExists').resolves(true);
+
+        // Mock the Python run function
+        const helpers = require('../../../managers/builtin/helpers');
+        sinon.stub(helpers, 'isUvInstalled').resolves(false);
+        sinon.stub(helpers, 'runPython').resolves();
+
+        await createWithProgress(
+            mockNativeFinder,
+            mockApi as unknown as PythonEnvironmentApi,
+            mockLog,
+            mockManager,
+            mockBasePython,
+            Uri.file('/test'),
+            '/test/venv',
+            { install: ['numpy'], uninstall: [] },
+        );
+
+        // Verify managePackages was called with suppressProgress:true
+        assert(mockManagePackages.called, 'managePackages should have been called');
+        const managePackagesCall = mockManagePackages.getCall(0);
+        assert.strictEqual(
+            managePackagesCall.args[1].suppressProgress,
+            true,
+            'managePackages should be called with suppressProgress:true',
+        );
+    });
+
+    test('should not call managePackages when no packages to install', async () => {
+        // Mock file system check
+        const fsapi = require('fs-extra');
+        sinon.stub(fsapi, 'pathExists').resolves(true);
+
+        // Mock the Python run function
+        const helpers = require('../../../managers/builtin/helpers');
+        sinon.stub(helpers, 'isUvInstalled').resolves(false);
+        sinon.stub(helpers, 'runPython').resolves();
+
+        await createWithProgress(
+            mockNativeFinder,
+            mockApi as unknown as PythonEnvironmentApi,
+            mockLog,
+            mockManager,
+            mockBasePython,
+            Uri.file('/test'),
+            '/test/venv',
+            { install: [], uninstall: [] },
+        );
+
+        // Verify managePackages was NOT called when there are no packages
+        assert(!mockManagePackages.called, 'managePackages should not be called when no packages to install');
+    });
+});