Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/9490.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Disable use of `conda run`.
1 change: 1 addition & 0 deletions news/2 Fixes/9496.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Revert changes related to calling `mypy` with relative paths.
25 changes: 4 additions & 21 deletions src/client/common/process/pythonExecutionFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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!;
}

Expand All @@ -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;
Expand Down Expand Up @@ -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([
Expand Down
5 changes: 1 addition & 4 deletions src/client/linters/mypy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as path from 'path';
import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
import '../common/extensions';
import { Product } from '../common/types';
Expand All @@ -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;
Expand Down
10 changes: 8 additions & 2 deletions src/test/common/process/condaExecutionService.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down
21 changes: 17 additions & 4 deletions src/test/common/process/pythonExecutionFactory.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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';
Expand Down
3 changes: 1 addition & 2 deletions src/test/linters/lint.args.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
141 changes: 37 additions & 104 deletions src/test/linters/mypy.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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');
Expand All @@ -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([]);
});
});