Add tests for the new API method

Author: Kartik Raj

src/client/common/interpreterPathService.ts

@@ -139,9 +139,15 @@ export class InterpreterPathService implements IInterpreterPathService {
 
     public async copyOldInterpreterStorageValuesToNew(resource: Resource): Promise<void> {
         resource = PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService).uri;
-        const workspaceConfig = this.workspaceService.getConfiguration('python', resource);
-        const oldSettings = workspaceConfig.inspect<string>('pythonPath')!;
+        const oldSettings = this.workspaceService.getConfiguration('python', resource).inspect<string>('pythonPath')!;
+        await Promise.all([
+            this._copyWorkspaceFolderValueToNewStorage(resource, oldSettings.workspaceFolderValue),
+            this._copyWorkspaceValueToNewStorage(resource, oldSettings.workspaceValue),
+            this._moveGlobalSettingValueToNewStorage(oldSettings.globalValue)
+        ]);
+    }
 
+    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 flaggedWorkspaceFolderKeysStorage = this.persistentStateFactory.createGlobalPersistentState<string[]>(
@@ -151,35 +157,44 @@ export class InterpreterPathService implements IInterpreterPathService {
         const flaggedWorkspaceFolderKeys = flaggedWorkspaceFolderKeysStorage.value;
         const shouldUpdateWorkspaceFolderSetting = !flaggedWorkspaceFolderKeys.includes(workspaceFolderKey);
         if (shouldUpdateWorkspaceFolderSetting) {
-            await this.update(resource, ConfigurationTarget.WorkspaceFolder, oldSettings.workspaceFolderValue);
+            await this.update(resource, ConfigurationTarget.WorkspaceFolder, value);
             await flaggedWorkspaceFolderKeysStorage.updateValue([workspaceFolderKey, ...flaggedWorkspaceFolderKeys]);
         }
+    }
 
+    public async _copyWorkspaceValueToNewStorage(resource: Resource, value: string | undefined): Promise<void> {
         // Copy workspace setting into the new storage if it hasn't been copied already
         const workspaceKey = this.workspaceService.workspaceFile
             ? this.fileSystemPaths.normCase(this.workspaceService.workspaceFile.fsPath)
             : undefined;
+        if (!workspaceKey) {
+            return;
+        }
         const flaggedWorkspaceKeysStorage = this.persistentStateFactory.createGlobalPersistentState<string[]>(
             workspaceKeysForWhichTheCopyIsDone_Key,
             []
         );
         const flaggedWorkspaceKeys = flaggedWorkspaceKeysStorage.value;
-        const shouldUpdateWorkspaceSetting = workspaceKey && !flaggedWorkspaceKeys.includes(workspaceKey);
-        if (workspaceKey && shouldUpdateWorkspaceSetting) {
-            await this.update(resource, ConfigurationTarget.Workspace, oldSettings.workspaceValue);
+        const shouldUpdateWorkspaceSetting = !flaggedWorkspaceKeys.includes(workspaceKey);
+        if (shouldUpdateWorkspaceSetting) {
+            await this.update(resource, ConfigurationTarget.Workspace, value);
             await flaggedWorkspaceKeysStorage.updateValue([workspaceKey, ...flaggedWorkspaceKeys]);
         }
+    }
 
+    public async _moveGlobalSettingValueToNewStorage(value: string | undefined) {
         // Move global setting into the new storage if it hasn't been moved already
         const isGlobalSettingCopiedStorage = this.persistentStateFactory.createGlobalPersistentState<boolean>(
             isGlobalSettingCopiedKey,
             false
         );
         const shouldUpdateGlobalSetting = !isGlobalSettingCopiedStorage.value;
         if (shouldUpdateGlobalSetting) {
-            await this.update(undefined, ConfigurationTarget.Global, oldSettings.globalValue);
+            await this.update(undefined, ConfigurationTarget.Global, value);
             // Make sure to delete the original setting after copying it
-            await workspaceConfig.update('pythonPath', undefined, ConfigurationTarget.Global);
+            await this.workspaceService
+                .getConfiguration('python')
+                .update('pythonPath', undefined, ConfigurationTarget.Global);
             await isGlobalSettingCopiedStorage.updateValue(true);
         }
     }

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

@@ -18,12 +18,14 @@ import { IWorkspaceService } from '../../client/common/application/types';
 import {
     defaultInterpreterPathSetting,
     InterpreterPathService,
+    isGlobalSettingCopiedKey,
+    workspaceFolderKeysForWhichTheCopyIsDone_Key,
     workspaceKeysForWhichTheCopyIsDone_Key
 } from '../../client/common/interpreterPathService';
 import { InterpreterConfigurationScope, IPersistentState, IPersistentStateFactory } from '../../client/common/types';
 import { createDeferred, sleep } from '../../client/common/utils/async';
 
-suite('xInterpreter Path Service', async () => {
+suite('Interpreter Path Service', async () => {
     let interpreterPathService: InterpreterPathService;
     let persistentStateFactory: TypeMoq.IMock<IPersistentStateFactory>;
     let workspaceService: TypeMoq.IMock<IWorkspaceService>;
@@ -50,8 +52,19 @@ suite('xInterpreter Path Service', async () => {
         sinon.restore();
     });
 
-    test('If the one-off transfer to new storage has not happened yet for the workspace, do it and record the transfer', async () => {
-        const update = sinon.stub(InterpreterPathService.prototype, 'update');
+    test('Ensure execution of method copyOldInterpreterStorageValuesToNew() goes as expected', async () => {
+        const _copyWorkspaceFolderValueToNewStorage = sinon.stub(
+            InterpreterPathService.prototype,
+            '_copyWorkspaceFolderValueToNewStorage'
+        );
+        const _copyWorkspaceValueToNewStorage = sinon.stub(
+            InterpreterPathService.prototype,
+            '_copyWorkspaceValueToNewStorage'
+        );
+        const _moveGlobalSettingValueToNewStorage = sinon.stub(
+            InterpreterPathService.prototype,
+            '_moveGlobalSettingValueToNewStorage'
+        );
         const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
         workspaceService.setup((w) => w.getConfiguration('python', resource)).returns(() => workspaceConfig.object);
         workspaceConfig
@@ -65,54 +78,144 @@ suite('xInterpreter Path Service', async () => {
                         // tslint:disable-next-line: no-any
                     } as any)
             );
+
+        interpreterPathService = new InterpreterPathService(persistentStateFactory.object, workspaceService.object, []);
+        await interpreterPathService.copyOldInterpreterStorageValuesToNew(resource);
+
+        assert(_copyWorkspaceFolderValueToNewStorage.calledWith(resource, 'workspaceFolderPythonPath'));
+        assert(_copyWorkspaceValueToNewStorage.calledWith(resource, 'workspacePythonPath'));
+        assert(_moveGlobalSettingValueToNewStorage.calledWith('globalPythonPath'));
+    });
+
+    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);
+        persistentStateFactory
+            .setup((p) => p.createGlobalPersistentState<string[]>(workspaceFolderKeysForWhichTheCopyIsDone_Key, []))
+            .returns(() => persistentState.object);
+        persistentState.setup((p) => p.value).returns(() => ['...storedWorkspaceFolderKeys']);
+        persistentState
+            .setup((p) => p.updateValue([resource.fsPath, '...storedWorkspaceFolderKeys']))
+            .returns(() => Promise.resolve())
+            .verifiable(TypeMoq.Times.once());
+
+        interpreterPathService = new InterpreterPathService(persistentStateFactory.object, workspaceService.object, []);
+        await interpreterPathService._copyWorkspaceFolderValueToNewStorage(resource, 'workspaceFolderPythonPath');
+
+        assert(update.calledWith(resource, ConfigurationTarget.WorkspaceFolder, 'workspaceFolderPythonPath'));
+        persistentState.verifyAll();
+    });
+
+    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);
+        persistentStateFactory
+            .setup((p) => p.createGlobalPersistentState<string[]>(workspaceFolderKeysForWhichTheCopyIsDone_Key, []))
+            .returns(() => persistentState.object);
+        persistentState.setup((p) => p.value).returns(() => [resource.fsPath, '...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 = 'PATH\\TO\\WORKSPACEFILE';
+        const update = sinon.stub(InterpreterPathService.prototype, 'update');
+        const persistentState = TypeMoq.Mock.ofType<IPersistentState<string[]>>();
+        workspaceService.setup((w) => w.workspaceFile).returns(() => workspaceFileUri);
         persistentStateFactory
             .setup((p) => p.createGlobalPersistentState<string[]>(workspaceKeysForWhichTheCopyIsDone_Key, []))
             .returns(() => persistentState.object);
         persistentState.setup((p) => p.value).returns(() => ['...storedWorkspaceKeys']);
         persistentState
-            .setup((p) => p.updateValue([resource.fsPath, '...storedWorkspaceKeys']))
+            .setup((p) => p.updateValue([expectedWorkspaceKey, '...storedWorkspaceKeys']))
             .returns(() => Promise.resolve())
             .verifiable(TypeMoq.Times.once());
 
         interpreterPathService = new InterpreterPathService(persistentStateFactory.object, workspaceService.object, []);
-        await interpreterPathService.copyOldInterpreterStorageValuesToNew(resource);
+        await interpreterPathService._copyWorkspaceValueToNewStorage(resource, 'workspacePythonPath');
 
-        update.calledWith(resource, ConfigurationTarget.WorkspaceFolder, 'workspaceFolderPythonPath');
-        update.calledWith(resource, ConfigurationTarget.Workspace, 'workspacePythonPath');
-        update.calledWith(undefined, ConfigurationTarget.Global, 'globalPythonPath');
+        assert(update.calledWith(resource, ConfigurationTarget.Workspace, 'workspacePythonPath'));
         persistentState.verifyAll();
     });
 
     test('If the one-off transfer to new storage has already happened for the workspace, do not update and simply return', async () => {
+        const workspaceFileUri = Uri.parse('path/to/workspaceFile');
+        const expectedWorkspaceKey = 'PATH\\TO\\WORKSPACEFILE';
         const update = sinon.stub(InterpreterPathService.prototype, 'update');
-        const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
-        workspaceService.setup((w) => w.getConfiguration('python', resource)).returns(() => workspaceConfig.object);
-        workspaceConfig
-            .setup((w) => w.inspect<string>('pythonPath'))
-            .returns(
-                () =>
-                    ({
-                        globalValue: 'globalPythonPath',
-                        workspaceFolderValue: 'workspaceFolderPythonPath',
-                        workspaceValue: 'workspacePythonPath'
-                        // tslint:disable-next-line: no-any
-                    } as any)
-            );
         const persistentState = TypeMoq.Mock.ofType<IPersistentState<string[]>>();
-        workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource)).returns(() => resource.fsPath);
+        workspaceService.setup((w) => w.workspaceFile).returns(() => workspaceFileUri);
         persistentStateFactory
             .setup((p) => p.createGlobalPersistentState<string[]>(workspaceKeysForWhichTheCopyIsDone_Key, []))
             .returns(() => persistentState.object);
-        persistentState.setup((p) => p.value).returns(() => [resource.fsPath, '...storedWorkspaceKeys']);
-        persistentState
-            .setup((p) => p.updateValue(TypeMoq.It.isAny()))
+        persistentState.setup((p) => p.value).returns(() => [expectedWorkspaceKey, '...storedWorkspaceKeys']);
+        persistentState.setup((p) => p.updateValue(TypeMoq.It.isAny())).verifiable(TypeMoq.Times.never());
+
+        interpreterPathService = new InterpreterPathService(persistentStateFactory.object, workspaceService.object, []);
+        await interpreterPathService._copyWorkspaceValueToNewStorage(resource, 'workspacePythonPath');
+
+        assert(update.notCalled);
+        persistentState.verifyAll();
+    });
+
+    test('Do not update workspace settings and if a folder is directly opened', async () => {
+        const update = sinon.stub(InterpreterPathService.prototype, 'update');
+        const persistentState = TypeMoq.Mock.ofType<IPersistentState<string[]>>();
+        workspaceService.setup((w) => w.workspaceFile).returns(() => undefined);
+        persistentStateFactory
+            .setup((p) => p.createGlobalPersistentState<string[]>(workspaceKeysForWhichTheCopyIsDone_Key, []))
+            .returns(() => persistentState.object);
+        persistentState.setup((p) => p.value).verifiable(TypeMoq.Times.never());
+        persistentState.setup((p) => p.updateValue(TypeMoq.It.isAny())).verifiable(TypeMoq.Times.never());
+
+        interpreterPathService = new InterpreterPathService(persistentStateFactory.object, workspaceService.object, []);
+        await interpreterPathService._copyWorkspaceValueToNewStorage(resource, 'workspacePythonPath');
+
+        assert(update.notCalled);
+        persistentState.verifyAll();
+    });
+
+    test('If the one-off transfer to new storage has not happened yet for the user setting, do it, record the transfer and remove the original user setting', async () => {
+        const update = sinon.stub(InterpreterPathService.prototype, 'update');
+        const persistentState = TypeMoq.Mock.ofType<IPersistentState<boolean>>();
+        persistentStateFactory
+            .setup((p) => p.createGlobalPersistentState<boolean>(isGlobalSettingCopiedKey, false))
+            .returns(() => persistentState.object);
+        persistentState.setup((p) => p.value).returns(() => false);
+        persistentState.setup((p) => p.updateValue(true)).verifiable(TypeMoq.Times.once());
+        const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
+        workspaceService.setup((w) => w.getConfiguration('python')).returns(() => workspaceConfig.object);
+        workspaceConfig
+            .setup((w) => w.update('pythonPath', undefined, ConfigurationTarget.Global))
             .returns(() => Promise.resolve())
-            .verifiable(TypeMoq.Times.never());
+            .verifiable(TypeMoq.Times.once());
 
         interpreterPathService = new InterpreterPathService(persistentStateFactory.object, workspaceService.object, []);
-        await interpreterPathService.copyOldInterpreterStorageValuesToNew(resource);
+        await interpreterPathService._moveGlobalSettingValueToNewStorage('globalPythonPath');
+
+        assert(update.calledWith(undefined, ConfigurationTarget.Global, 'globalPythonPath'));
+        persistentState.verifyAll();
+        workspaceConfig.verifyAll();
+    });
+
+    test('If the one-off transfer to new storage has already happened for the user setting, do not update and simply return', async () => {
+        const update = sinon.stub(InterpreterPathService.prototype, 'update');
+        const persistentState = TypeMoq.Mock.ofType<IPersistentState<boolean>>();
+        persistentStateFactory
+            .setup((p) => p.createGlobalPersistentState<boolean>(isGlobalSettingCopiedKey, false))
+            .returns(() => persistentState.object);
+        persistentState.setup((p) => p.value).returns(() => true);
+        persistentState.setup((p) => p.updateValue(TypeMoq.It.isAny())).verifiable(TypeMoq.Times.never());
+
+        interpreterPathService = new InterpreterPathService(persistentStateFactory.object, workspaceService.object, []);
+        await interpreterPathService._moveGlobalSettingValueToNewStorage('globalPythonPath');
 
         assert(update.notCalled);
         persistentState.verifyAll();