update tests

Author: eleanorjboyd

src/managers/builtin/venvUtils.ts

@@ -63,25 +63,28 @@ export async function clearVenvCache(): Promise<void> {
 export async function getVenvForWorkspace(fsPath: string): Promise<string | undefined> {
     // Check persisted user selection first — this should always take priority
     // over process.env.VIRTUAL_ENV so that explicit selections survive reload.
-    const state = await getWorkspacePersistentState();
-    const data: { [key: string]: string } | undefined = await state.get(VENV_WORKSPACE_KEY);
-    if (data) {
-        try {
+    try {
+        const state = await getWorkspacePersistentState();
+        const data: { [key: string]: string } | undefined = await state.get(VENV_WORKSPACE_KEY);
+        if (data) {
             const envPath = data[fsPath];
             if (envPath && (await fsapi.pathExists(envPath))) {
                 return envPath;
             }
             if (envPath) {
                 await setVenvForWorkspace(fsPath, undefined);
             }
-        } catch {
-            // fall through to VIRTUAL_ENV check
         }
+    } catch {
+        // fall through to VIRTUAL_ENV check
     }
 
     // Fall back to VIRTUAL_ENV only if it points to a path inside this workspace.
-    if (process.env.VIRTUAL_ENV && normalizePath(process.env.VIRTUAL_ENV).startsWith(normalizePath(fsPath))) {
-        return process.env.VIRTUAL_ENV;
+    if (process.env.VIRTUAL_ENV) {
+        const rel = path.relative(fsPath, process.env.VIRTUAL_ENV);
+        if (rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel))) {
+            return process.env.VIRTUAL_ENV;
+        }
     }
 
     return undefined;

src/test/managers/builtin/venvUtils.getVenvForWorkspace.unit.test.ts

@@ -6,6 +6,96 @@ import * as sinon from 'sinon';
 import * as persistentState from '../../../common/persistentState';
 import { getVenvForWorkspace, VENV_WORKSPACE_KEY } from '../../../managers/builtin/venvUtils';
 
+/**
+ * Helper that replicates the VIRTUAL_ENV subpath check from getVenvForWorkspace
+ * using a specific path module, allowing cross-platform verification.
+ */
+function isVenvInsideWorkspace(
+    fsPath: string,
+    virtualEnv: string,
+    pathModule: typeof path.posix | typeof path.win32,
+): boolean {
+    const rel = pathModule.relative(fsPath, virtualEnv);
+    return rel === '' || (!rel.startsWith('..') && !pathModule.isAbsolute(rel));
+}
+
+suite('VIRTUAL_ENV subpath check - cross-platform', () => {
+    suite('POSIX paths', () => {
+        const p = path.posix;
+
+        test('venv inside workspace should match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('/proj/app', '/proj/app/.venv', p), true);
+        });
+
+        test('venv deeply nested inside workspace should match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('/proj/app', '/proj/app/sub/dir/.venv', p), true);
+        });
+
+        test('venv equal to workspace should match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('/proj/app', '/proj/app', p), true);
+        });
+
+        test('sibling with shared prefix should NOT match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('/proj/app', '/proj/app2/.venv', p), false);
+        });
+
+        test('venv in completely different location should NOT match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('/proj/app', '/other/place/.venv', p), false);
+        });
+
+        test('parent directory should NOT match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('/proj/app', '/proj/.venv', p), false);
+        });
+    });
+
+    suite('Windows paths', () => {
+        const p = path.win32;
+
+        test('venv inside workspace should match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('C:\\proj\\app', 'C:\\proj\\app\\.venv', p), true);
+        });
+
+        test('venv deeply nested inside workspace should match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('C:\\proj\\app', 'C:\\proj\\app\\sub\\dir\\.venv', p), true);
+        });
+
+        test('venv equal to workspace should match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('C:\\proj\\app', 'C:\\proj\\app', p), true);
+        });
+
+        test('sibling with shared prefix should NOT match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('C:\\proj\\app', 'C:\\proj\\app2\\.venv', p), false);
+        });
+
+        test('venv on a different drive should NOT match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('C:\\proj\\app', 'D:\\proj\\app\\.venv', p), false);
+        });
+
+        test('parent directory should NOT match', () => {
+            assert.strictEqual(isVenvInsideWorkspace('C:\\proj\\app', 'C:\\proj\\.venv', p), false);
+        });
+
+        test('case-insensitive drive letters should match', () => {
+            // path.win32.relative handles case-insensitive drive letters
+            assert.strictEqual(isVenvInsideWorkspace('c:\\proj\\app', 'C:\\proj\\app\\.venv', p), true);
+        });
+
+        test('UNC path inside workspace should match', () => {
+            assert.strictEqual(
+                isVenvInsideWorkspace('\\\\server\\share\\proj', '\\\\server\\share\\proj\\.venv', p),
+                true,
+            );
+        });
+
+        test('UNC path sibling should NOT match', () => {
+            assert.strictEqual(
+                isVenvInsideWorkspace('\\\\server\\share\\proj', '\\\\server\\share\\proj2\\.venv', p),
+                false,
+            );
+        });
+    });
+});
+
 suite('getVenvForWorkspace', () => {
     let mockState: {
         get: sinon.SinonStub;
@@ -163,4 +253,41 @@ suite('getVenvForWorkspace', () => {
 
         assert.ok(!mockState.set.called, 'Should not call set when there is no stale entry to clear');
     });
+
+    test('should fall back to VIRTUAL_ENV when state.get rejects', async () => {
+        const workspacePath = path.join(tmpDir, 'projectA');
+        const virtualEnvPath = path.join(workspacePath, '.venv');
+        await fse.ensureDir(virtualEnvPath);
+        process.env.VIRTUAL_ENV = virtualEnvPath;
+
+        mockState.get.withArgs(VENV_WORKSPACE_KEY).rejects(new Error('storage corrupted'));
+
+        const result = await getVenvForWorkspace(workspacePath);
+        assert.strictEqual(result, virtualEnvPath, 'Should fall back to VIRTUAL_ENV when state.get rejects');
+    });
+
+    test('should fall back to VIRTUAL_ENV when getWorkspacePersistentState rejects', async () => {
+        const workspacePath = path.join(tmpDir, 'projectA');
+        const virtualEnvPath = path.join(workspacePath, '.venv');
+        await fse.ensureDir(virtualEnvPath);
+        process.env.VIRTUAL_ENV = virtualEnvPath;
+
+        getWorkspacePersistentStateStub.rejects(new Error('persistent state unavailable'));
+
+        const result = await getVenvForWorkspace(workspacePath);
+        assert.strictEqual(result, virtualEnvPath, 'Should fall back to VIRTUAL_ENV when persistent state fails');
+    });
+
+    test('should NOT use VIRTUAL_ENV for sibling path with shared prefix', async () => {
+        const projectA = path.join(tmpDir, 'app');
+        const siblingVenv = path.join(tmpDir, 'app2', '.venv');
+        await fse.ensureDir(projectA);
+        await fse.ensureDir(siblingVenv);
+        process.env.VIRTUAL_ENV = siblingVenv;
+
+        mockState.get.withArgs(VENV_WORKSPACE_KEY).resolves(undefined);
+
+        const result = await getVenvForWorkspace(projectA);
+        assert.strictEqual(result, undefined, 'Should NOT match sibling path with shared prefix');
+    });
 });