From 61a1e7f8d186255f1dcd41790b3851190a3dd803 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sun, 7 Jun 2020 08:21:31 -0700 Subject: [PATCH 1/2] Ensure extension features are started when in Deprecate PythonPath experiment and opening a file without any folder opened. --- news/2 Fixes/12177.md | 1 + src/client/common/interpreterPathService.ts | 9 ++++- .../interpreterPathService.unit.test.ts | 39 +++++++++++-------- 3 files changed, 31 insertions(+), 18 deletions(-) create mode 100644 news/2 Fixes/12177.md diff --git a/news/2 Fixes/12177.md b/news/2 Fixes/12177.md new file mode 100644 index 000000000000..cbd8364e8fca --- /dev/null +++ b/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. diff --git a/src/client/common/interpreterPathService.ts b/src/client/common/interpreterPathService.ts index f06cf1dd15e3..41a7571ecb28 100644 --- a/src/client/common/interpreterPathService.ts +++ b/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( @@ -149,7 +151,10 @@ export class InterpreterPathService implements IInterpreterPathService { public async _copyWorkspaceFolderValueToNewStorage(resource: Resource, value: string | undefined): Promise { // 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 === '') { + return; + } const flaggedWorkspaceFolderKeysStorage = this.persistentStateFactory.createGlobalPersistentState( workspaceFolderKeysForWhichTheCopyIsDone_Key, [] diff --git a/src/test/common/interpreterPathService.unit.test.ts b/src/test/common/interpreterPathService.unit.test.ts index a824b2223812..ad4aa91b1e4c 100644 --- a/src/test/common/interpreterPathService.unit.test.ts +++ b/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>(); - workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource)).returns(() => resource.fsPath); + workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource, '')).returns(() => resource.fsPath); persistentStateFactory .setup((p) => p.createGlobalPersistentState(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>(); - workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource)).returns(() => resource.fsPath); + workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource, '')).returns(() => resource.fsPath); persistentStateFactory .setup((p) => p.createGlobalPersistentState(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>(); + workspaceService.setup((w) => w.getWorkspaceFolderIdentifier(resource, '')).returns(() => ''); + persistentStateFactory + .setup((p) => p.createGlobalPersistentState(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>(); 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>(); 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(); From 208db836edbe26048dd667838d976f9e4d95d4f6 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sun, 7 Jun 2020 08:23:55 -0700 Subject: [PATCH 2/2] Added comments --- src/client/common/interpreterPathService.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/common/interpreterPathService.ts b/src/client/common/interpreterPathService.ts index 41a7571ecb28..a5bbc17108b5 100644 --- a/src/client/common/interpreterPathService.ts +++ b/src/client/common/interpreterPathService.ts @@ -153,6 +153,7 @@ export class InterpreterPathService implements IInterpreterPathService { // Copy workspace folder setting into the new storage if it hasn't been copied already const workspaceFolderKey = this.workspaceService.getWorkspaceFolderIdentifier(resource, ''); if (workspaceFolderKey === '') { + // No workspace folder is opened, simply return. return; } const flaggedWorkspaceFolderKeysStorage = this.persistentStateFactory.createGlobalPersistentState(