Ensure extension features are started when in Deprecate PythonPath experiment and opening a file without any folder opened (#12182)

* Ensure extension features are started when in Deprecate PythonPath experiment and opening a file without any folder opened.

* Added comments

Author: Kartik Raj

news/2 Fixes/12177.md

@@ -0,0 +1 @@
+Ensure extension features are started when in `Deprecate PythonPath` experiment and opening a file without any folder opened.

src/client/common/interpreterPathService.ts

@@ -9,6 +9,7 @@ import { ConfigurationChangeEvent, ConfigurationTarget, Event, EventEmitter, Uri
 import { IWorkspaceService } from './application/types';
 import { PythonSettings } from './configSettings';
 import { isTestExecution } from './constants';
+import { traceError } from './logger';
 import { FileSystemPaths } from './platform/fs-paths';
 import {
     IDisposable,
@@ -105,7 +106,8 @@ export class InterpreterPathService implements IInterpreterPathService {
             return;
         }
         if (!resource) {
-            throw new Error('Cannot update workspace settings as no workspace is opened');
+            traceError('Cannot update workspace settings as no workspace is opened');
+            return;
         }
         const settingKey = this.getSettingKey(resource, configTarget);
         const persistentSetting = this.persistentStateFactory.createGlobalPersistentState<string | undefined>(
@@ -149,7 +151,11 @@ export class InterpreterPathService implements IInterpreterPathService {
 
     public async _copyWorkspaceFolderValueToNewStorage(resource: Resource, value: string | undefined): Promise<void> {
         // Copy workspace folder setting into the new storage if it hasn't been copied already
-        const workspaceFolderKey = this.workspaceService.getWorkspaceFolderIdentifier(resource);
+        const workspaceFolderKey = this.workspaceService.getWorkspaceFolderIdentifier(resource, '');
+        if (workspaceFolderKey === '') {
+            // No workspace folder is opened, simply return.
+            return;
+        }
         const flaggedWorkspaceFolderKeysStorage = this.persistentStateFactory.createGlobalPersistentState<string[]>(
             workspaceFolderKeysForWhichTheCopyIsDone_Key,
             []

src/test/common/interpreterPathService.unit.test.ts

@@ -92,7 +92,7 @@ suite('Interpreter Path Service', async () => {
     test('If the one-off transfer to new storage has not happened yet for the workspace folder, do it and record the transfer', async () => {
         const update = sinon.stub(InterpreterPathService.prototype, 'update');
         const persistentState = TypeMoq.Mock.ofType<IPersistentState<string[]>>();
-        workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource)).returns(() => resource.fsPath);
+        workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource, '')).returns(() => resource.fsPath);
         persistentStateFactory
             .setup((p) => p.createGlobalPersistentState<string[]>(workspaceFolderKeysForWhichTheCopyIsDone_Key, []))
             .returns(() => persistentState.object);
@@ -112,7 +112,7 @@ suite('Interpreter Path Service', async () => {
     test('If the one-off transfer to new storage has already happened for the workspace folder, do not update and simply return', async () => {
         const update = sinon.stub(InterpreterPathService.prototype, 'update');
         const persistentState = TypeMoq.Mock.ofType<IPersistentState<string[]>>();
-        workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource)).returns(() => resource.fsPath);
+        workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource, '')).returns(() => resource.fsPath);
         persistentStateFactory
             .setup((p) => p.createGlobalPersistentState<string[]>(workspaceFolderKeysForWhichTheCopyIsDone_Key, []))
             .returns(() => persistentState.object);
@@ -126,6 +126,23 @@ suite('Interpreter Path Service', async () => {
         persistentState.verifyAll();
     });
 
+    test('If no folder is opened, do not do the one-off transfer to new storage for the workspace folder', async () => {
+        const update = sinon.stub(InterpreterPathService.prototype, 'update');
+        const persistentState = TypeMoq.Mock.ofType<IPersistentState<string[]>>();
+        workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource, '')).returns(() => '');
+        persistentStateFactory
+            .setup((p) => p.createGlobalPersistentState<string[]>(workspaceFolderKeysForWhichTheCopyIsDone_Key, []))
+            .returns(() => persistentState.object);
+        persistentState.setup((p) => p.value).returns(() => ['...storedWorkspaceKeys']);
+        persistentState.setup((p) => p.updateValue(TypeMoq.It.isAny())).verifiable(TypeMoq.Times.never());
+
+        interpreterPathService = new InterpreterPathService(persistentStateFactory.object, workspaceService.object, []);
+        await interpreterPathService._copyWorkspaceFolderValueToNewStorage(resource, 'workspaceFolderPythonPath');
+
+        assert(update.notCalled);
+        persistentState.verifyAll();
+    });
+
     test('If the one-off transfer to new storage has not happened yet for the workspace, do it and record the transfer', async () => {
         const workspaceFileUri = Uri.parse('path/to/workspaceFile');
         const expectedWorkspaceKey = fs.normCase(workspaceFileUri.fsPath);
@@ -395,7 +412,7 @@ suite('Interpreter Path Service', async () => {
         _didChangeInterpreterEmitter.verifyAll();
     });
 
-    test('Updating workspace settings throws error if no workspace is opened', async () => {
+    test('Updating workspace settings simply returns if no workspace is opened', async () => {
         const expectedSettingKey = `WORKSPACE_FOLDER_INTERPRETER_PATH_${resource.fsPath}`;
         const persistentState = TypeMoq.Mock.ofType<IPersistentState<string | undefined>>();
         workspaceService.setup((w) => w.workspaceFolders).returns(() => undefined);
@@ -408,18 +425,13 @@ suite('Interpreter Path Service', async () => {
             .returns(() => Promise.resolve())
             .verifiable(TypeMoq.Times.never());
 
-        const promise = interpreterPathService.update(
-            resourceOutsideOfWorkspace,
-            ConfigurationTarget.Workspace,
-            interpreterPath
-        );
-        await expect(promise).to.eventually.be.rejectedWith(Error);
+        await interpreterPathService.update(resourceOutsideOfWorkspace, ConfigurationTarget.Workspace, interpreterPath);
 
         persistentState.verifyAll();
         persistentStateFactory.verifyAll();
     });
 
-    test('Updating workspace folder settings throws error if no workspace is opened', async () => {
+    test('Updating workspace folder settings simply returns if no workspace is opened', async () => {
         const expectedSettingKey = `WORKSPACE_FOLDER_INTERPRETER_PATH_${resource.fsPath}`;
         const persistentState = TypeMoq.Mock.ofType<IPersistentState<string | undefined>>();
         workspaceService.setup((w) => w.workspaceFolders).returns(() => undefined);
@@ -432,12 +444,7 @@ suite('Interpreter Path Service', async () => {
             .returns(() => Promise.resolve())
             .verifiable(TypeMoq.Times.never());
 
-        const promise = interpreterPathService.update(
-            resourceOutsideOfWorkspace,
-            ConfigurationTarget.Workspace,
-            interpreterPath
-        );
-        await expect(promise).to.eventually.be.rejectedWith(Error);
+        await interpreterPathService.update(resourceOutsideOfWorkspace, ConfigurationTarget.Workspace, interpreterPath);
 
         persistentState.verifyAll();
         persistentStateFactory.verifyAll();