From 86a649239a8b80f835c271d644c5df2cf2ff42d5 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 9 Jan 2020 13:04:30 -0800 Subject: [PATCH 1/6] Disable conda execution service instantiation --- .../common/process/pythonExecutionFactory.ts | 21 ++----------------- .../condaExecutionService.unit.test.ts | 10 +++++++-- .../pythonExecutionFactory.unit.test.ts | 13 +++++++++--- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index 8626cd33c7ca..f153bb750e3b 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -52,17 +52,6 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { const processLogger = this.serviceContainer.get(IProcessLogger); processService.on('exec', processLogger.logProcess.bind(processLogger)); - // Don't bother getting a conda execution service instance if we haven't fetched the list of interpreters yet. - // Also, without this hasInterpreters check smoke tests will time out - const interpreterService = this.serviceContainer.get(IInterpreterService); - const hasInterpreters = await interpreterService.hasInterpreters; - if (hasInterpreters) { - const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService); - if (condaExecutionService) { - return condaExecutionService; - } - } - if (this.windowsStoreInterpreter.isWindowsStoreInterpreter(pythonPath)) { return new WindowsStorePythonProcess(this.serviceContainer, processService, pythonPath, this.windowsStoreInterpreter); } @@ -128,16 +117,10 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { processService.on('exec', processLogger.logProcess.bind(processLogger)); this.serviceContainer.get(IDisposableRegistry).push(processService); - // Allow parts of the code to ignore conda run. - if (!options.bypassCondaExecution) { - const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService); - if (condaExecutionService) { - return condaExecutionService; - } - } - return new PythonExecutionService(this.serviceContainer, processService, pythonPath); } + // Not using CondaExecutionService instances since there are breaking issues with conda run (as of PVSC 2020.1) + // See https://github.com/microsoft/vscode-python/issues/9490 public async createCondaExecutionService(pythonPath: string, processService?: IProcessService, resource?: Uri): Promise { const processServicePromise = processService ? Promise.resolve(processService) : this.processServiceFactory.create(resource); const [condaVersion, condaEnvironment, condaFile, procService] = await Promise.all([ diff --git a/src/test/common/process/condaExecutionService.unit.test.ts b/src/test/common/process/condaExecutionService.unit.test.ts index 8ef07c408684..6956a17d333e 100644 --- a/src/test/common/process/condaExecutionService.unit.test.ts +++ b/src/test/common/process/condaExecutionService.unit.test.ts @@ -24,7 +24,10 @@ suite('CondaExecutionService', () => { serviceContainer.setup(s => s.get(IFileSystem)).returns(() => fileSystem.object); }); - test('getExecutionInfo with a named environment should return execution info using the environment name', () => { + test('getExecutionInfo with a named environment should return execution info using the environment name', function() { + // tslint:disable-next-line:no-invalid-this + return this.skip(); + const environment = { name: 'foo', path: 'bar' }; executionService = new CondaExecutionService(serviceContainer.object, processService.object, pythonPath, condaFile, environment); @@ -33,7 +36,10 @@ suite('CondaExecutionService', () => { expect(result).to.deep.equal({ command: condaFile, args: ['run', '-n', environment.name, 'python', ...args] }); }); - test('getExecutionInfo with a non-named environment should return execution info using the environment path', async () => { + test('getExecutionInfo with a non-named environment should return execution info using the environment path', async function() { + // tslint:disable-next-line:no-invalid-this + return this.skip(); + const environment = { name: '', path: 'bar' }; executionService = new CondaExecutionService(serviceContainer.object, processService.object, pythonPath, condaFile, environment); diff --git a/src/test/common/process/pythonExecutionFactory.unit.test.ts b/src/test/common/process/pythonExecutionFactory.unit.test.ts index e857df37e4b9..a8926e0b5c5e 100644 --- a/src/test/common/process/pythonExecutionFactory.unit.test.ts +++ b/src/test/common/process/pythonExecutionFactory.unit.test.ts @@ -253,7 +253,10 @@ suite('Process - PythonExecutionFactory', () => { expect(service).instanceOf(PythonExecutionService); }); - test('Ensure `createActivatedEnvironment` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async () => { + test('Ensure `createActivatedEnvironment` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async function() { + // tslint:disable-next-line:no-invalid-this + return this.skip(); + const pythonPath = 'path/to/python'; const pythonSettings = mock(PythonSettings); @@ -268,17 +271,21 @@ suite('Process - PythonExecutionFactory', () => { const service = await factory.createActivatedEnvironment({ resource, interpreter }); verify(condaService.getCondaFile()).once(); - if (!interpreter) { + if (interpreter) { verify(pythonSettings.pythonPath).once(); verify(condaService.getCondaEnvironment(pythonPath)).once(); } else { + // @ts-ignore verify(condaService.getCondaEnvironment(interpreter.path)).once(); } expect(service).instanceOf(CondaExecutionService); }); - test('Ensure `createActivatedEnvironment` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async () => { + test('Ensure `createActivatedEnvironment` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function() { + // tslint:disable-next-line:no-invalid-this + return this.skip(); + let createInvoked = false; const pythonPath = 'path/to/python'; const mockExecService = 'mockService'; From 0f2bbf7acd6b5948d53939a1eb55b42827429d38 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 9 Jan 2020 13:06:36 -0800 Subject: [PATCH 2/6] News file --- news/2 Fixes/9490.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/9490.md diff --git a/news/2 Fixes/9490.md b/news/2 Fixes/9490.md new file mode 100644 index 000000000000..c61cc3450333 --- /dev/null +++ b/news/2 Fixes/9490.md @@ -0,0 +1 @@ +Disable use of `conda run`. From ad259625a29e743f2838e70bd817d369ec29690f Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 9 Jan 2020 13:09:07 -0800 Subject: [PATCH 3/6] Didn't mean to commit that --- src/test/common/process/pythonExecutionFactory.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/process/pythonExecutionFactory.unit.test.ts b/src/test/common/process/pythonExecutionFactory.unit.test.ts index a8926e0b5c5e..d4b632ff8ecf 100644 --- a/src/test/common/process/pythonExecutionFactory.unit.test.ts +++ b/src/test/common/process/pythonExecutionFactory.unit.test.ts @@ -271,7 +271,7 @@ suite('Process - PythonExecutionFactory', () => { const service = await factory.createActivatedEnvironment({ resource, interpreter }); verify(condaService.getCondaFile()).once(); - if (interpreter) { + if (!interpreter) { verify(pythonSettings.pythonPath).once(); verify(condaService.getCondaEnvironment(pythonPath)).once(); } else { From e1c43b31908bd19f2ad950d2ec00d8db838d11f2 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 9 Jan 2020 13:36:09 -0800 Subject: [PATCH 4/6] Forgot a few tests --- .../common/process/pythonExecutionFactory.unit.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/common/process/pythonExecutionFactory.unit.test.ts b/src/test/common/process/pythonExecutionFactory.unit.test.ts index d4b632ff8ecf..090367511111 100644 --- a/src/test/common/process/pythonExecutionFactory.unit.test.ts +++ b/src/test/common/process/pythonExecutionFactory.unit.test.ts @@ -212,7 +212,10 @@ suite('Process - PythonExecutionFactory', () => { expect(service).instanceOf(WindowsStorePythonProcess); }); - test('Ensure `create` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async () => { + test('Ensure `create` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async function() { + // tslint:disable-next-line:no-invalid-this + return this.skip(); + const pythonPath = 'path/to/python'; const pythonSettings = mock(PythonSettings); @@ -234,7 +237,10 @@ suite('Process - PythonExecutionFactory', () => { expect(service).instanceOf(CondaExecutionService); }); - test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async () => { + test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function () { + // tslint:disable-next-line:no-invalid-this + return this.skip(); + const pythonPath = 'path/to/python'; const pythonSettings = mock(PythonSettings); when(processFactory.create(resource)).thenResolve(processService.object); From 396aae7503dbcf2c84d4b9a1c2fb15f70a59ad6c Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 9 Jan 2020 13:45:34 -0800 Subject: [PATCH 5/6] Linting --- src/test/common/process/pythonExecutionFactory.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/process/pythonExecutionFactory.unit.test.ts b/src/test/common/process/pythonExecutionFactory.unit.test.ts index 090367511111..86a3471ab00d 100644 --- a/src/test/common/process/pythonExecutionFactory.unit.test.ts +++ b/src/test/common/process/pythonExecutionFactory.unit.test.ts @@ -237,7 +237,7 @@ suite('Process - PythonExecutionFactory', () => { expect(service).instanceOf(CondaExecutionService); }); - test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function () { + test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function() { // tslint:disable-next-line:no-invalid-this return this.skip(); From dc46b2d6f95e95ccbd166c5083ee29957b481f11 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 9 Jan 2020 14:00:17 -0800 Subject: [PATCH 6/6] Update comment --- src/client/common/process/pythonExecutionFactory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index f153bb750e3b..05498f1c2d45 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -119,7 +119,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { return new PythonExecutionService(this.serviceContainer, processService, pythonPath); } - // Not using CondaExecutionService instances since there are breaking issues with conda run (as of PVSC 2020.1) + // Not using this function for now because there are breaking issues with conda run (conda 4.8, PVSC 2020.1). // See https://github.com/microsoft/vscode-python/issues/9490 public async createCondaExecutionService(pythonPath: string, processService?: IProcessService, resource?: Uri): Promise { const processServicePromise = processService ? Promise.resolve(processService) : this.processServiceFactory.create(resource);