Cherry pick conda and mypy into release-2019.12 (#9530)

kimadeline · web-flow · commit 68951437af98 · 2020-01-10T11:22:50.000-08:00
* Revert calling mypy with relatives paths (regression) (#9502) * Revert "fix(client/linters/mypy): call mypy incorrectly (#5834)" This reverts commit 1b6fbfb. * News file * Revert "Fix MyPy CI tests (#8518)" This reverts commit c634ffd. * Disable conda execution service instantiation (#9493) * Disable conda execution service instantiation * News file * Didn't mean to commit that * Forgot a few tests * Linting * Update comment * Linting * Test failure caused by linting * Undo tslint formatting change
diff --git a/news/2 Fixes/9490.md b/news/2 Fixes/9490.md
@@ -0,0 +1 @@
+Disable use of `conda run`.
diff --git a/news/2 Fixes/9496.md b/news/2 Fixes/9496.md
@@ -0,0 +1 @@
+Revert changes related to calling `mypy` with relative paths.
diff --git 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>(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>(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);
         }
@@ -83,7 +72,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
 
         // No daemon support in Python 2.7.
         const interpreter = await interpreterService.getInterpreterDetails(pythonPath);
-        if (interpreter?.version && interpreter.version.major < 3){
+        if (interpreter?.version && interpreter.version.major < 3) {
             return activatedProcPromise!;
         }
 
@@ -95,7 +84,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
                 this.activationHelper.getActivatedEnvironmentVariables(options.resource, interpreter, true)
             ]);
 
-            const daemon = new PythonDaemonExecutionServicePool(logger, disposables, {...options, pythonPath}, activatedProc!, activatedEnvVars);
+            const daemon = new PythonDaemonExecutionServicePool(logger, disposables, { ...options, pythonPath }, activatedProc!, activatedEnvVars);
             await daemon.initialize();
             disposables.push(daemon);
             return daemon;
@@ -128,16 +117,10 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         processService.on('exec', processLogger.logProcess.bind(processLogger));
         this.serviceContainer.get<IDisposableRegistry>(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 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<CondaExecutionService | undefined> {
         const processServicePromise = processService ? Promise.resolve(processService) : this.processServiceFactory.create(resource);
         const [condaVersion, condaEnvironment, condaFile, procService] = await Promise.all([
diff --git a/src/client/linters/mypy.ts b/src/client/linters/mypy.ts
@@ -1,4 +1,3 @@
-import * as path from 'path';
 import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
 import '../common/extensions';
 import { Product } from '../common/types';
@@ -14,9 +13,7 @@ export class MyPy extends BaseLinter {
     }
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
-        const cwd = this.getWorkspaceRootPath(document);
-        const relativePath = path.relative(cwd, document.uri.fsPath);
-        const messages = await this.run([relativePath], document, cancellation, REGEX);
+        const messages = await this.run([document.uri.fsPath], document, cancellation, REGEX);
         messages.forEach(msg => {
             msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.mypyCategorySeverity);
             msg.code = msg.type;
diff --git 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>(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
@@ -196,7 +196,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);
 
@@ -218,7 +221,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);
@@ -237,7 +243,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);
 
@@ -256,13 +265,17 @@ suite('Process - PythonExecutionFactory', () => {
                     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';
diff --git a/src/test/linters/lint.args.test.ts b/src/test/linters/lint.args.test.ts
@@ -137,8 +137,7 @@ suite('Linting - Arguments', () => {
                 });
                 test('MyPy', async () => {
                     const linter = new MyPy(outputChannel.object, serviceContainer);
-                    const expectedPath = workspaceUri ? path.join(path.basename(path.dirname(fileUri.fsPath)), path.basename(fileUri.fsPath)) : path.basename(fileUri.fsPath);
-                    const expectedArgs = [expectedPath];
+                    const expectedArgs = [fileUri.fsPath];
                     await testLinter(linter, expectedArgs);
                 });
                 test('Pydocstyle', async () => {
diff --git a/src/test/linters/mypy.unit.test.ts b/src/test/linters/mypy.unit.test.ts
@@ -6,20 +6,12 @@
 // tslint:disable:no-object-literal-type-assertion
 
 import { expect } from 'chai';
-import * as path from 'path';
-import * as sinon from 'sinon';
-import { anything, instance, mock, when } from 'ts-mockito';
-import { CancellationToken, CancellationTokenSource, TextDocument, Uri } from 'vscode';
-import { Product } from '../../client/common/types';
-import { ServiceContainer } from '../../client/ioc/container';
 import { parseLine } from '../../client/linters/baseLinter';
-import { LinterManager } from '../../client/linters/linterManager';
-import { MyPy, REGEX } from '../../client/linters/mypy';
-import { ILinterManager, ILintMessage, LintMessageSeverity } from '../../client/linters/types';
-import { MockOutputChannel } from '../mockClasses';
+import { REGEX } from '../../client/linters/mypy';
+import { ILintMessage } from '../../client/linters/types';
 
 // This following is a real-world example. See gh=2380.
-// tslint:disable:no-multiline-string no-any max-func-body-length
+// tslint:disable-next-line:no-multiline-string
 const output = `
 provider.pyi:10: error: Incompatible types in assignment (expression has type "str", variable has type "int")
 provider.pyi:11: error: Name 'not_declared_var' is not defined
@@ -30,30 +22,40 @@ suite('Linting - MyPy', () => {
     test('regex', async () => {
         const lines = output.split('\n');
         const tests: [string, ILintMessage][] = [
-            [lines[1], {
-                code: undefined,
-                message: 'Incompatible types in assignment (expression has type "str", variable has type "int")',
-                column: 0,
-                line: 10,
-                type: 'error',
-                provider: 'mypy'
-            } as ILintMessage],
-            [lines[2], {
-                code: undefined,
-                message: 'Name \'not_declared_var\' is not defined',
-                column: 0,
-                line: 11,
-                type: 'error',
-                provider: 'mypy'
-            } as ILintMessage],
-            [lines[3], {
-                code: undefined,
-                message: 'Expression has type "Any"',
-                column: 21,
-                line: 12,
-                type: 'error',
-                provider: 'mypy'
-            } as ILintMessage]
+            [
+                lines[1],
+                {
+                    code: undefined,
+                    message: 'Incompatible types in assignment (expression has type "str", variable has type "int")',
+                    column: 0,
+                    line: 10,
+                    type: 'error',
+                    provider: 'mypy'
+                } as ILintMessage
+            ],
+            [
+                lines[2],
+                {
+                    code: undefined,
+                    // tslint:disable-next-line
+                    message: "Name 'not_declared_var' is not defined",
+                    column: 0,
+                    line: 11,
+                    type: 'error',
+                    provider: 'mypy'
+                } as ILintMessage
+            ],
+            [
+                lines[3],
+                {
+                    code: undefined,
+                    message: 'Expression has type "Any"',
+                    column: 21,
+                    line: 12,
+                    type: 'error',
+                    provider: 'mypy'
+                } as ILintMessage
+            ]
         ];
         for (const [line, expected] of tests) {
             const msg = parseLine(line, REGEX, 'mypy');
@@ -62,72 +64,3 @@ suite('Linting - MyPy', () => {
         }
     });
 });
-
-suite('Test Linter', () => {
-    class TestMyPyLinter extends MyPy {
-        // tslint:disable: no-unnecessary-override
-        public async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
-            return super.runLinter(document, cancellation);
-        }
-        public getWorkspaceRootPath(document: TextDocument): string {
-            return super.getWorkspaceRootPath(document);
-        }
-        public async run(args: string[], document: TextDocument, cancellation: CancellationToken, regEx: string = REGEX): Promise<ILintMessage[]> {
-            return super.run(args, document, cancellation, regEx);
-        }
-        public parseMessagesSeverity(error: string, severity: any): LintMessageSeverity {
-            return super.parseMessagesSeverity(error, severity);
-        }
-    }
-
-    let linter: TestMyPyLinter;
-    let getWorkspaceRootPathStub: sinon.SinonStub<[TextDocument], string>;
-    let runStub: sinon.SinonStub<[string[], TextDocument, CancellationToken, (string | undefined)?], Promise<ILintMessage[]>>;
-    const token = new CancellationTokenSource().token;
-    teardown(() => sinon.restore());
-    setup(() => {
-        const linterManager = mock(LinterManager);
-        when(linterManager.getLinterInfo(anything())).thenReturn({ product: Product.mypy } as any);
-        const serviceContainer = mock(ServiceContainer);
-        when(serviceContainer.get<ILinterManager>(ILinterManager)).thenReturn(instance(linterManager));
-        getWorkspaceRootPathStub = sinon.stub(TestMyPyLinter.prototype, 'getWorkspaceRootPath');
-        runStub = sinon.stub(TestMyPyLinter.prototype, 'run');
-        linter = new TestMyPyLinter(instance(mock(MockOutputChannel)), instance(serviceContainer));
-    });
-
-    test('Get cwd based on document', async () => {
-        const fileUri = Uri.file(path.join('a', 'b', 'c', 'd', 'e', 'filename.py'));
-        const cwd = path.join('a', 'b', 'c');
-        const doc = { uri: fileUri } as any as TextDocument;
-        getWorkspaceRootPathStub.callsFake(() => cwd);
-        runStub.callsFake(() => Promise.resolve([]));
-
-        await linter.runLinter(doc, token);
-
-        expect(getWorkspaceRootPathStub.callCount).to.equal(1);
-        expect(getWorkspaceRootPathStub.args[0]).to.deep.equal([doc]);
-    });
-    test('Pass relative path of document to linter', async () => {
-        const fileUri = Uri.file(path.join('a', 'b', 'c', 'd', 'e', 'filename.py'));
-        const cwd = path.join('a', 'b', 'c');
-        const doc = { uri: fileUri } as any as TextDocument;
-        getWorkspaceRootPathStub.callsFake(() => cwd);
-        runStub.callsFake(() => Promise.resolve([]));
-
-        await linter.runLinter(doc, token);
-
-        expect(runStub.callCount).to.equal(1);
-        expect(runStub.args[0]).to.deep.equal([[path.relative(cwd, fileUri.fsPath)], doc, token, REGEX]);
-    });
-    test('Return empty messages', async () => {
-        const fileUri = Uri.file(path.join('a', 'b', 'c', 'd', 'e', 'filename.py'));
-        const cwd = path.join('a', 'b', 'c');
-        const doc = { uri: fileUri } as any as TextDocument;
-        getWorkspaceRootPathStub.callsFake(() => cwd);
-        runStub.callsFake(() => Promise.resolve([]));
-
-        const messages = await linter.runLinter(doc, token);
-
-        expect(messages).to.be.deep.equal([]);
-    });
-});

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Revert changes related to calling `mypy` with relative paths.