Add conda run without output

news/1 Enhancements/7696.md

@@ -0,0 +1 @@
+Add support for conda run without output, using `--no-capture-output` flag.

src/client/common/installer/productInstaller.ts

@@ -94,6 +94,7 @@ abstract class BaseInstaller {
         resource?: InterpreterUri,
         cancel?: CancellationToken,
         flags?: ModuleInstallFlags,
+        bypassCondaExecution?: boolean,
     ): Promise<InstallerResponse> {
         if (product === Product.unittest) {
             return InstallerResponse.Installed;
@@ -113,7 +114,7 @@ abstract class BaseInstaller {
             .installModule(product, resource, cancel, flags)
             .catch((ex) => traceError(`Error in installing the product '${ProductNames.get(product)}', ${ex}`));
 
-        return this.isInstalled(product, resource).then((isInstalled) => {
+        return this.isInstalled(product, resource, bypassCondaExecution).then((isInstalled) => {
             sendTelemetryEvent(EventName.PYTHON_INSTALL_PACKAGE, undefined, {
                 installer: installer.displayName,
                 productName: ProductNames.get(product),
@@ -178,7 +179,11 @@ abstract class BaseInstaller {
         }
     }
 
-    public async isInstalled(product: Product, resource?: InterpreterUri): Promise<boolean> {
+    public async isInstalled(
+        product: Product,
+        resource?: InterpreterUri,
+        bypassCondaExecution?: boolean,
+    ): Promise<boolean> {
         if (product === Product.unittest) {
             return true;
         }
@@ -191,7 +196,12 @@ abstract class BaseInstaller {
         if (isModule) {
             const pythonProcess = await this.serviceContainer
                 .get<IPythonExecutionFactory>(IPythonExecutionFactory)
-                .createActivatedEnvironment({ resource: uri, interpreter, allowEnvironmentFetchExceptions: true });
+                .createActivatedEnvironment({
+                    resource: uri,
+                    interpreter,
+                    allowEnvironmentFetchExceptions: true,
+                    bypassCondaExecution,
+                });
             return pythonProcess.isModuleInstalled(executableName);
         }
         const process = await this.serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory).create(uri);
@@ -595,12 +605,17 @@ export class ProductInstaller implements IInstaller {
         resource?: InterpreterUri,
         cancel?: CancellationToken,
         flags?: ModuleInstallFlags,
+        bypassCondaExecution?: boolean,
     ): Promise<InstallerResponse> {
-        return this.createInstaller(product).install(product, resource, cancel, flags);
+        return this.createInstaller(product).install(product, resource, cancel, flags, bypassCondaExecution);
     }
 
-    public async isInstalled(product: Product, resource?: InterpreterUri): Promise<boolean> {
-        return this.createInstaller(product).isInstalled(product, resource);
+    public async isInstalled(
+        product: Product,
+        resource?: InterpreterUri,
+        bypassCondaExecution?: boolean,
+    ): Promise<boolean> {
+        return this.createInstaller(product).isInstalled(product, resource, bypassCondaExecution);
     }
 
     // eslint-disable-next-line class-methods-use-this

src/client/common/process/pythonEnvironment.ts

@@ -145,15 +145,11 @@ export function createCondaEnv(
     } else {
         runArgs.push('-n', condaInfo.name);
     }
-    const pythonArgv = [condaFile, ...runArgs, 'python'];
+    const pythonArgv = [condaFile, ...runArgs, '--no-capture-output', 'python'];
     const deps = createDeps(
         async (filename) => fs.pathExists(filename),
         pythonArgv,
-
-        // TODO: Use pythonArgv here once 'conda run' can be
-        // run without buffering output.
-        // See https://github.com/microsoft/vscode-python/issues/8473.
-        undefined,
+        pythonArgv,
         (file, args, opts) => procs.exec(file, args, opts),
         (command, opts) => procs.shellExec(command, opts),
     );

src/client/common/process/pythonExecutionFactory.ts

@@ -29,8 +29,8 @@ import { IInterpreterAutoSelectionService } from '../../interpreter/autoSelectio
 import { sleep } from '../utils/async';
 import { traceError } from '../../logging';
 
-// Minimum version number of conda required to be able to use 'conda run'
-export const CONDA_RUN_VERSION = '4.6.0';
+// Minimum version number of conda required to be able to use 'conda run' and '--no-capture--output' option
+export const CONDA_RUN_VERSION = '4.9.0';
 
 @injectable()
 export class PythonExecutionFactory implements IPythonExecutionFactory {
@@ -83,6 +83,14 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         }
         const processService: IProcessService = await this.processServiceFactory.create(options.resource);
 
+        // Allow parts of the code to ignore conda run.
+        if (!options.bypassCondaExecution) {
+            const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService);
+            if (condaExecutionService) {
+                return condaExecutionService;
+            }
+        }
+
         const windowsStoreInterpreterCheck = this.pyenvs.isWindowsStoreInterpreter.bind(this.pyenvs);
 
         return createPythonService(
@@ -108,6 +116,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
             return this.create({
                 resource: options.resource,
                 pythonPath: options.interpreter ? options.interpreter.path : undefined,
+                bypassCondaExecution: options.bypassCondaExecution,
             });
         }
         const pythonPath = options.interpreter
@@ -117,11 +126,17 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         processService.on('exec', this.logger.logProcess.bind(this.logger));
         this.disposables.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 createPythonService(pythonPath, processService, this.fileSystem);
     }
 
-    // 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,
@@ -144,6 +159,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
                 procService.on('exec', this.logger.logProcess.bind(this.logger));
                 this.disposables.push(procService);
             }
+
             return createPythonService(
                 pythonPath,
                 procService,

src/client/common/process/types.ts

@@ -65,6 +65,7 @@ export const IPythonExecutionFactory = Symbol('IPythonExecutionFactory');
 export type ExecutionFactoryCreationOptions = {
     resource?: Uri;
     pythonPath?: string;
+    bypassCondaExecution?: boolean;
 };
 export type ExecutionFactoryCreateWithEnvironmentOptions = {
     resource?: Uri;

src/client/common/types.ts

@@ -121,8 +121,9 @@ export interface IInstaller {
         resource?: InterpreterUri,
         cancel?: CancellationToken,
         flags?: ModuleInstallFlags,
+        bypassCondaExecution?: boolean,
     ): Promise<InstallerResponse>;
-    isInstalled(product: Product, resource?: InterpreterUri): Promise<boolean>;
+    isInstalled(product: Product, resource?: InterpreterUri, bypassCondaExecution?: boolean): Promise<boolean>;
     isProductVersionCompatible(
         product: Product,
         semVerRequirement: string,

src/client/linters/flake8.ts

@@ -14,7 +14,7 @@ export class Flake8 extends BaseLinter {
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
         const messages = await this.run(
-            ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
+            ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
             document,
             cancellation,
         );

src/client/linters/pycodestyle.ts

@@ -14,7 +14,7 @@ export class Pycodestyle extends BaseLinter {
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
         const messages = await this.run(
-            ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
+            ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
             document,
             cancellation,
         );

src/test/common/installer.test.ts

@@ -296,7 +296,7 @@ suite('Installer', () => {
             }
             callback({ stdout: '' });
         });
-        await installer.isInstalled(product, resource);
+        await installer.isInstalled(product, resource, true);
         await checkInstalledDef.promise;
     }
     getNamesAndValues<Product>(Product).forEach((prod) => {
@@ -333,7 +333,7 @@ suite('Installer', () => {
                 checkInstalledDef.resolve();
             }
         });
-        await installer.install(product);
+        await installer.install(product, undefined, undefined, undefined, true);
         await checkInstalledDef.promise;
     }
     getNamesAndValues<Product>(Product).forEach((prod) => {

src/test/common/process/pythonEnvironment.unit.test.ts

@@ -295,8 +295,8 @@ suite('CondaEnvironment', () => {
 
         expect(result).to.deep.equal({
             command: condaFile,
-            args: ['run', '-n', condaInfo.name, 'python', ...args],
-            python: [condaFile, 'run', '-n', condaInfo.name, 'python'],
+            args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', ...args],
+            python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python'],
             pythonExecutable: 'python',
         });
     });
@@ -309,25 +309,35 @@ suite('CondaEnvironment', () => {
 
         expect(result).to.deep.equal({
             command: condaFile,
-            args: ['run', '-p', condaInfo.path, 'python', ...args],
-            python: [condaFile, 'run', '-p', condaInfo.path, 'python'],
+            args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', ...args],
+            python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python'],
             pythonExecutable: 'python',
         });
     });
 
-    test('getExecutionObservableInfo with a named environment should return execution info using pythonPath only', () => {
-        const expected = { command: pythonPath, args, python: [pythonPath], pythonExecutable: pythonPath };
+    test('getExecutionObservableInfo with a named environment should return execution info using conda full path with the name', () => {
         const condaInfo = { name: 'foo', path: 'bar' };
+        const expected = {
+            command: condaFile,
+            args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', ...args],
+            python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python'],
+            pythonExecutable: 'python',
+        };
         const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
 
         const result = env.getExecutionObservableInfo(args);
 
         expect(result).to.deep.equal(expected);
     });
 
-    test('getExecutionObservableInfo with a non-named environment should return execution info using pythonPath only', () => {
-        const expected = { command: pythonPath, args, python: [pythonPath], pythonExecutable: pythonPath };
+    test('getExecutionObservableInfo with a non-named environment should return execution info using conda full path', () => {
         const condaInfo = { name: '', path: 'bar' };
+        const expected = {
+            command: condaFile,
+            args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', ...args],
+            python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python'],
+            pythonExecutable: 'python',
+        };
         const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
 
         const result = env.getExecutionObservableInfo(args);

src/test/common/process/pythonExecutionFactory.unit.test.ts

@@ -257,9 +257,7 @@ suite('Process - PythonExecutionFactory', () => {
                 assert.strictEqual(createInvoked, false);
             });
 
-            test('Ensure `create` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async function () {
-                return this.skip();
-
+            test('Ensure `create` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async () => {
                 const pythonPath = 'path/to/python';
                 const pythonSettings = mock(PythonSettings);
 
@@ -284,9 +282,7 @@ suite('Process - PythonExecutionFactory', () => {
                 verify(condaService.getCondaFile()).once();
             });
 
-            test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function () {
-                return this.skip();
-
+            test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async () => {
                 const pythonPath = 'path/to/python';
                 const pythonSettings = mock(PythonSettings);
                 when(processFactory.create(resource)).thenResolve(processService.object);
@@ -305,9 +301,7 @@ suite('Process - PythonExecutionFactory', () => {
                 verify(condaService.getCondaFile()).once();
             });
 
-            test('Ensure `createActivatedEnvironment` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async function () {
-                return this.skip();
-
+            test('Ensure `createActivatedEnvironment` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async () => {
                 const pythonPath = 'path/to/python';
                 const pythonSettings = mock(PythonSettings);
 
@@ -336,9 +330,7 @@ suite('Process - PythonExecutionFactory', () => {
                 }
             });
 
-            test('Ensure `createActivatedEnvironment` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function () {
-                return this.skip();
-
+            test('Ensure `createActivatedEnvironment` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async () => {
                 let createInvoked = false;
                 const pythonPath = 'path/to/python';
                 const mockExecService = 'mockService';

src/test/format/extension.sort.test.ts

@@ -107,7 +107,7 @@ suite('Sorting', () => {
         await window.showTextDocument(textDocument);
         await commands.executeCommand(Commands.Sort_Imports);
         assert.notStrictEqual(originalContent, textDocument.getText(), 'Contents have not changed');
-    });
+    }).timeout(TEST_TIMEOUT * 3);
 
     test('With Config', async () => {
         const textDocument = await workspace.openTextDocument(fileToFormatWithConfig);
@@ -130,7 +130,7 @@ suite('Sorting', () => {
         await window.showTextDocument(textDocument);
         await commands.executeCommand(Commands.Sort_Imports);
         assert.notStrictEqual(originalContent, textDocument.getText(), 'Contents have not changed');
-    });
+    }).timeout(TEST_TIMEOUT * 3);
 
     test('With Changes and Config in Args', async () => {
         await updateSetting(
@@ -164,7 +164,7 @@ suite('Sorting', () => {
         const originalContent = textDocument.getText();
         await commands.executeCommand(Commands.Sort_Imports);
         assert.notStrictEqual(originalContent, textDocument.getText(), 'Contents have not changed');
-    }).timeout(TEST_TIMEOUT * 2);
+    }).timeout(TEST_TIMEOUT * 3);
 
     test('With Changes and Config implicit from cwd', async () => {
         const textDocument = await workspace.openTextDocument(fileToFormatWithConfig);

src/test/linters/lint.args.test.ts

@@ -142,12 +142,12 @@ suite('Linting - Arguments', () => {
                     }
                     test('Flake8', async () => {
                         const linter = new Flake8(serviceContainer);
-                        const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
+                        const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
                         await testLinter(linter, expectedArgs);
                     });
                     test('Pycodestyle', async () => {
                         const linter = new Pycodestyle(serviceContainer);
-                        const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
+                        const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
                         await testLinter(linter, expectedArgs);
                     });
                     test('Prospector', async () => {

src/test/linters/lint.functional.test.ts

@@ -625,6 +625,11 @@ class TestFixture extends BaseTestFixture {
         const serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>(undefined, TypeMoq.MockBehavior.Strict);
         const configService = TypeMoq.Mock.ofType<IConfigurationService>(undefined, TypeMoq.MockBehavior.Strict);
         const processLogger = TypeMoq.Mock.ofType<IProcessLogger>(undefined, TypeMoq.MockBehavior.Strict);
+        const componentAdapter = TypeMoq.Mock.ofType<IComponentAdapter>(undefined, TypeMoq.MockBehavior.Strict);
+        componentAdapter
+            .setup((c) => c.getCondaEnvironment(TypeMoq.It.isAny()))
+            .returns(() => Promise.resolve(undefined));
+
         const filesystem = new FileSystem();
         processLogger
             .setup((p) => p.logProcess(TypeMoq.It.isAnyString(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
@@ -637,6 +642,9 @@ class TestFixture extends BaseTestFixture {
         serviceContainer
             .setup((s) => s.get(TypeMoq.It.isValue(IFileSystem), TypeMoq.It.isAny()))
             .returns(() => filesystem);
+        serviceContainer
+            .setup((s) => s.get(TypeMoq.It.isValue(IComponentAdapter), TypeMoq.It.isAny()))
+            .returns(() => componentAdapter.object);
 
         const platformService = new PlatformService();