test: harden package integration retries

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e

Author: edvilme

src/test/common/testUtils.unit.test.ts

@@ -80,6 +80,28 @@ suite('Test Utilities', () => {
                 (error: unknown) => error === conditionError,
             );
         });
+
+        test('should retry and preserve the last condition error at timeout', async () => {
+            const conditionError = new Error('Package refresh failed');
+            let attempts = 0;
+
+            await assert.rejects(
+                () =>
+                    waitForCondition(
+                        () => {
+                            attempts++;
+                            return Promise.reject(conditionError);
+                        },
+                        50,
+                        'Should preserve the refresh error',
+                        10,
+                        true,
+                        true,
+                    ),
+                (error: unknown) => error === conditionError,
+            );
+            assert.ok(attempts > 1, 'The rejected condition should be retried before timing out');
+        });
     });
 
     suite('retryUntilSuccess', () => {

src/test/integration/environmentFixture.ts

@@ -75,9 +75,19 @@ export async function createEnvironmentFixture(
     let projectSettingAdded = false;
     let environmentCreated = false;
     let environment: PythonEnvironment | undefined;
+    let apiRemovalSettled = false;
     let disposePromise: Promise<void> | undefined;
     let markerWritten = false;
     let projectRootCreated = false;
+    const getApiRemoval = createSingleFlightOperation(() => {
+        if (!environment) {
+            return Promise.resolve();
+        }
+        apiRemovalSettled = false;
+        return api.removeEnvironment(environment, { runHeadless: true }).finally(() => {
+            apiRemovalSettled = true;
+        });
+    });
 
     const cleanup = async (): Promise<void> => {
         const cleanupErrors: Error[] = [];
@@ -92,15 +102,10 @@ export async function createEnvironmentFixture(
                 cleanupErrors.push(toError(error));
             }
             if (ownershipVerified && environment) {
-                let apiRemovalSettled = false;
-                const apiRemoval = api
-                    .removeEnvironment(environment, { runHeadless: true })
-                    .finally(() => {
-                        apiRemovalSettled = true;
-                    });
+                const apiRemovalPromise = getApiRemoval();
                 try {
                     await withTimeout(
-                        apiRemoval,
+                        apiRemovalPromise,
                         COMMAND_TIMEOUT_MS,
                         `${request.name} API environment removal timed out`,
                     );
@@ -109,7 +114,7 @@ export async function createEnvironmentFixture(
                     if (!apiRemovalSettled) {
                         try {
                             await withTimeout(
-                                apiRemoval,
+                                apiRemovalPromise,
                                 API_REMOVAL_SETTLE_TIMEOUT_MS,
                                 `${request.name} API environment removal did not settle after timing out`,
                             );
@@ -537,6 +542,17 @@ async function withTimeout<T>(operation: Promise<T>, timeoutMs: number, message:
     }
 }
 
+/**
+ * Returns a function that starts an asynchronous operation at most once and shares its promise.
+ */
+export function createSingleFlightOperation<T>(operation: () => Promise<T>): () => Promise<T> {
+    let promise: Promise<T> | undefined;
+    return () => {
+        promise ??= operation();
+        return promise;
+    };
+}
+
 function sanitizeName(value: string): string {
     return value.toLowerCase().replace(/[^a-z0-9-]/g, '-');
 }

src/test/integration/environmentFixture.unit.test.ts

@@ -0,0 +1,30 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import assert from 'assert';
+import { createSingleFlightOperation } from './environmentFixture';
+
+suite('Environment fixture helpers', () => {
+    test('shares a delayed operation across retries', async () => {
+        let calls = 0;
+        let resolveOperation: (() => void) | undefined;
+        const getOperation = createSingleFlightOperation(
+            () =>
+                new Promise<void>((resolve) => {
+                    calls++;
+                    resolveOperation = resolve;
+                }),
+        );
+
+        const first = getOperation();
+        const retry = getOperation();
+
+        assert.strictEqual(retry, first);
+        assert.strictEqual(calls, 1);
+        assert.ok(resolveOperation);
+        resolveOperation();
+        await Promise.all([first, retry]);
+        assert.strictEqual(getOperation(), first);
+        assert.strictEqual(calls, 1);
+    });
+});

src/test/integration/packageManager.integration.test.ts

@@ -154,7 +154,8 @@ for (const profile of profiles) {
                 30_000,
                 'Package not installed',
                 1_000,
-                false,
+                true,
+                true,
             );
 
             const directPackageNames = await vscode.commands.executeCommand<string[] | undefined>(
@@ -175,7 +176,8 @@ for (const profile of profiles) {
                     30_000,
                     'Package not uninstalled',
                     1_000,
-                    false,
+                    true,
+                    true,
                 );
             }
         });

src/test/testUtils.ts

@@ -29,6 +29,7 @@ export function sleep(ms: number): Promise<void> {
  * @param errorMessage - Error message if condition is not met
  * @param pollIntervalMs - How often to check condition (default: 100ms)
  * @param retryOnError - Whether rejected conditions should be retried (default: true)
+ * @param rejectWithLastError - Whether a timeout after rejected conditions should preserve the last error
  *
  * @example
  * // Wait for extension to activate
@@ -52,13 +53,16 @@ export async function waitForCondition(
     errorMessage: string | (() => string) = 'Condition not met within timeout',
     pollIntervalMs: number = 100,
     retryOnError: boolean = true,
+    rejectWithLastError: boolean = false,
 ): Promise<void> {
     return new Promise<void>((resolve, reject) => {
         const startTime = Date.now();
+        let lastError: unknown;
 
         const checkCondition = async () => {
             try {
                 const result = await condition();
+                lastError = undefined;
                 if (result) {
                     resolve();
                     return;
@@ -68,9 +72,14 @@ export async function waitForCondition(
                     reject(error);
                     return;
                 }
+                lastError = error;
             }
 
             if (Date.now() - startTime >= timeoutMs) {
+                if (rejectWithLastError && lastError !== undefined) {
+                    reject(lastError);
+                    return;
+                }
                 const msg = typeof errorMessage === 'function' ? errorMessage() : errorMessage;
                 reject(new Error(`${msg} (waited ${timeoutMs}ms)`));
                 return;