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`. diff --git a/news/2 Fixes/9496.md b/news/2 Fixes/9496.md new file mode 100644 index 000000000000..ec605ad8658d --- /dev/null +++ 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 index 184b9d19e1e2..05498f1c2d45 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); } @@ -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).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 { 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 index fb5de8442f4f..7c8559881a42 100644 --- 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 { - 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 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 fe35529939e8..3cc5a33c04e0 100644 --- 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 index b4f1982a844c..d922c416b40f 100644 --- 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 index 2dd0c01ba1a5..517316ce6bba 100644 --- 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 { - 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 { - 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>; - 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)).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([]); - }); -});