Skip to content

Commit 2f8ff40

Browse files
paulacamargo25Kartik Raj
authored andcommitted
Add conda run without output (no bypass) (microsoft/vscode-python#18319)
* Add conda run command * Fix code run * Fix linting * Fix tests * Add news * Fix tests in linter * Fix strings * Fix linters * Fix conda run unit test * Fix funtional tests * Fix single workspace tests * update minimum conda versionvalue * Change sorting timeput * Add timeout time in sorting test * Fix test wothout config * Add more time to timeout in sorting tests * Add conda run command * Fix code run * Fix linting * Fix tests * Add news * Fix tests in linter * Fix strings * Fix linters * Fix conda run unit test * Fix funtional tests * Fix single workspace tests * update minimum conda versionvalue * Change sorting timeput * Add timeout time in sorting test * Add more time to timeout in sorting tests * Update news/1 Enhancements/7696.md Co-authored-by: Kartik Raj <[email protected]> * Rix pylint test * Fix lint test * Remove unnecessary interpreter check * Fix timeout test errors * Fix unit test pip installer * Fix IModuleInstaller test * Remove bypass Conda * Remove bypass * Add timeout time * Add conda run command * Fix code run * Fix linting * Fix tests * Add news * Fix tests in linter * Fix strings * Fix linters * Fix conda run unit test * Fix funtional tests * Fix single workspace tests * update minimum conda versionvalue * Change sorting timeput * Add timeout time in sorting test * Add more time to timeout in sorting tests * Add conda run command * Fix code run * Fix test wothout config * Add more time to timeout in sorting tests * Update news/1 Enhancements/7696.md Co-authored-by: Kartik Raj <[email protected]> * Rix pylint test * Fix lint test * Remove unnecessary interpreter check * Fix timeout test errors * Fix unit test pip installer * Fix IModuleInstaller test * Remove bypass Conda * Remove bypass * Add timeout time * Fix compilation error * Fix compilation error * Fin pylint linter * Fix installer test Co-authored-by: Kartik Raj <[email protected]>
1 parent c24f64a commit 2f8ff40

File tree

12 files changed

+56
-41
lines changed

12 files changed

+56
-41
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add support for conda run without output, using `--no-capture-output` flag.

extensions/positron-python/src/client/common/process/pythonEnvironment.ts

+2-6
Original file line numberDiff line numberDiff line change
@@ -145,15 +145,11 @@ export function createCondaEnv(
145145
} else {
146146
runArgs.push('-n', condaInfo.name);
147147
}
148-
const pythonArgv = [condaFile, ...runArgs, 'python'];
148+
const pythonArgv = [condaFile, ...runArgs, '--no-capture-output', 'python'];
149149
const deps = createDeps(
150150
async (filename) => fs.pathExists(filename),
151151
pythonArgv,
152-
153-
// TODO: Use pythonArgv here once 'conda run' can be
154-
// run without buffering output.
155-
// See https://github.com/microsoft/vscode-python/issues/8473.
156-
undefined,
152+
pythonArgv,
157153
(file, args, opts) => procs.exec(file, args, opts),
158154
(command, opts) => procs.shellExec(command, opts),
159155
);

extensions/positron-python/src/client/common/process/pythonExecutionFactory.ts

+13-4
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ import { IInterpreterAutoSelectionService } from '../../interpreter/autoSelectio
2929
import { sleep } from '../utils/async';
3030
import { traceError } from '../../logging';
3131

32-
// Minimum version number of conda required to be able to use 'conda run'
33-
export const CONDA_RUN_VERSION = '4.6.0';
32+
// Minimum version number of conda required to be able to use 'conda run' and '--no-capture--output' option
33+
export const CONDA_RUN_VERSION = '4.9.0';
3434

3535
@injectable()
3636
export class PythonExecutionFactory implements IPythonExecutionFactory {
@@ -83,6 +83,11 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
8383
}
8484
const processService: IProcessService = await this.processServiceFactory.create(options.resource);
8585

86+
const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService);
87+
if (condaExecutionService) {
88+
return condaExecutionService;
89+
}
90+
8691
const windowsStoreInterpreterCheck = this.pyenvs.isWindowsStoreInterpreter.bind(this.pyenvs);
8792

8893
return createPythonService(
@@ -117,11 +122,14 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
117122
processService.on('exec', this.logger.logProcess.bind(this.logger));
118123
this.disposables.push(processService);
119124

125+
const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService);
126+
if (condaExecutionService) {
127+
return condaExecutionService;
128+
}
129+
120130
return createPythonService(pythonPath, processService, this.fileSystem);
121131
}
122132

123-
// Not using this function for now because there are breaking issues with conda run (conda 4.8, PVSC 2020.1).
124-
// See https://github.com/microsoft/vscode-python/issues/9490
125133
public async createCondaExecutionService(
126134
pythonPath: string,
127135
processService?: IProcessService,
@@ -144,6 +152,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
144152
procService.on('exec', this.logger.logProcess.bind(this.logger));
145153
this.disposables.push(procService);
146154
}
155+
147156
return createPythonService(
148157
pythonPath,
149158
procService,

extensions/positron-python/src/client/common/process/types.ts

-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ export type ExecutionFactoryCreateWithEnvironmentOptions = {
7676
*
7777
* @type {boolean}
7878
*/
79-
bypassCondaExecution?: boolean;
8079
};
8180
export interface IPythonExecutionFactory {
8281
create(options: ExecutionFactoryCreationOptions): Promise<IPythonExecutionService>;

extensions/positron-python/src/client/linters/flake8.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export class Flake8 extends BaseLinter {
1414

1515
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
1616
const messages = await this.run(
17-
['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
17+
['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
1818
document,
1919
cancellation,
2020
);

extensions/positron-python/src/client/linters/pycodestyle.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export class Pycodestyle extends BaseLinter {
1414

1515
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
1616
const messages = await this.run(
17-
['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
17+
['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
1818
document,
1919
cancellation,
2020
);

extensions/positron-python/src/test/common/installer.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ import { rootWorkspaceUri, updateSetting } from '../common';
115115
import { MockModuleInstaller } from '../mocks/moduleInstaller';
116116
import { MockProcessService } from '../mocks/proc';
117117
import { UnitTestIocContainer } from '../testing/serviceRegistry';
118-
import { closeActiveWindows, initializeTest, IS_MULTI_ROOT_TEST } from '../initialize';
118+
import { closeActiveWindows, initializeTest, IS_MULTI_ROOT_TEST, TEST_TIMEOUT } from '../initialize';
119119

120120
suite('Installer', () => {
121121
let ioc: UnitTestIocContainer;
@@ -313,7 +313,7 @@ suite('Installer', () => {
313313
await testCheckingIfProductIsInstalled(prod.value);
314314

315315
return undefined;
316-
});
316+
}).timeout(TEST_TIMEOUT * 3);
317317
});
318318

319319
async function testInstallingProduct(product: Product) {
@@ -351,6 +351,6 @@ suite('Installer', () => {
351351
await testInstallingProduct(prod.value);
352352

353353
return undefined;
354-
});
354+
}).timeout(TEST_TIMEOUT * 3);
355355
});
356356
});

extensions/positron-python/src/test/common/process/pythonEnvironment.unit.test.ts

+18-8
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,8 @@ suite('CondaEnvironment', () => {
287287

288288
expect(result).to.deep.equal({
289289
command: condaFile,
290-
args: ['run', '-n', condaInfo.name, 'python', ...args],
291-
python: [condaFile, 'run', '-n', condaInfo.name, 'python'],
290+
args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', ...args],
291+
python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python'],
292292
pythonExecutable: 'python',
293293
});
294294
});
@@ -301,25 +301,35 @@ suite('CondaEnvironment', () => {
301301

302302
expect(result).to.deep.equal({
303303
command: condaFile,
304-
args: ['run', '-p', condaInfo.path, 'python', ...args],
305-
python: [condaFile, 'run', '-p', condaInfo.path, 'python'],
304+
args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', ...args],
305+
python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python'],
306306
pythonExecutable: 'python',
307307
});
308308
});
309309

310-
test('getExecutionObservableInfo with a named environment should return execution info using pythonPath only', () => {
311-
const expected = { command: pythonPath, args, python: [pythonPath], pythonExecutable: pythonPath };
310+
test('getExecutionObservableInfo with a named environment should return execution info using conda full path with the name', () => {
312311
const condaInfo = { name: 'foo', path: 'bar' };
312+
const expected = {
313+
command: condaFile,
314+
args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', ...args],
315+
python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python'],
316+
pythonExecutable: 'python',
317+
};
313318
const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
314319

315320
const result = env.getExecutionObservableInfo(args);
316321

317322
expect(result).to.deep.equal(expected);
318323
});
319324

320-
test('getExecutionObservableInfo with a non-named environment should return execution info using pythonPath only', () => {
321-
const expected = { command: pythonPath, args, python: [pythonPath], pythonExecutable: pythonPath };
325+
test('getExecutionObservableInfo with a non-named environment should return execution info using conda full path', () => {
322326
const condaInfo = { name: '', path: 'bar' };
327+
const expected = {
328+
command: condaFile,
329+
args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', ...args],
330+
python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python'],
331+
pythonExecutable: 'python',
332+
};
323333
const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
324334

325335
const result = env.getExecutionObservableInfo(args);

extensions/positron-python/src/test/common/process/pythonExecutionFactory.unit.test.ts

+4-12
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,7 @@ suite('Process - PythonExecutionFactory', () => {
257257
assert.strictEqual(createInvoked, false);
258258
});
259259

260-
test('Ensure `create` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async function () {
261-
return this.skip();
262-
260+
test('Ensure `create` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async () => {
263261
const pythonPath = 'path/to/python';
264262
const pythonSettings = mock(PythonSettings);
265263

@@ -284,9 +282,7 @@ suite('Process - PythonExecutionFactory', () => {
284282
verify(condaService.getCondaFile()).once();
285283
});
286284

287-
test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function () {
288-
return this.skip();
289-
285+
test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async () => {
290286
const pythonPath = 'path/to/python';
291287
const pythonSettings = mock(PythonSettings);
292288
when(processFactory.create(resource)).thenResolve(processService.object);
@@ -305,9 +301,7 @@ suite('Process - PythonExecutionFactory', () => {
305301
verify(condaService.getCondaFile()).once();
306302
});
307303

308-
test('Ensure `createActivatedEnvironment` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async function () {
309-
return this.skip();
310-
304+
test('Ensure `createActivatedEnvironment` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async () => {
311305
const pythonPath = 'path/to/python';
312306
const pythonSettings = mock(PythonSettings);
313307

@@ -336,9 +330,7 @@ suite('Process - PythonExecutionFactory', () => {
336330
}
337331
});
338332

339-
test('Ensure `createActivatedEnvironment` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function () {
340-
return this.skip();
341-
333+
test('Ensure `createActivatedEnvironment` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async () => {
342334
let createInvoked = false;
343335
const pythonPath = 'path/to/python';
344336
const mockExecService = 'mockService';

extensions/positron-python/src/test/format/extension.sort.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ suite('Sorting', () => {
107107
await window.showTextDocument(textDocument);
108108
await commands.executeCommand(Commands.Sort_Imports);
109109
assert.notStrictEqual(originalContent, textDocument.getText(), 'Contents have not changed');
110-
});
110+
}).timeout(TEST_TIMEOUT * 3);
111111

112112
test('With Config', async () => {
113113
const textDocument = await workspace.openTextDocument(fileToFormatWithConfig);
@@ -130,7 +130,7 @@ suite('Sorting', () => {
130130
await window.showTextDocument(textDocument);
131131
await commands.executeCommand(Commands.Sort_Imports);
132132
assert.notStrictEqual(originalContent, textDocument.getText(), 'Contents have not changed');
133-
});
133+
}).timeout(TEST_TIMEOUT * 3);
134134

135135
test('With Changes and Config in Args', async () => {
136136
await updateSetting(
@@ -164,7 +164,7 @@ suite('Sorting', () => {
164164
const originalContent = textDocument.getText();
165165
await commands.executeCommand(Commands.Sort_Imports);
166166
assert.notStrictEqual(originalContent, textDocument.getText(), 'Contents have not changed');
167-
}).timeout(TEST_TIMEOUT * 2);
167+
}).timeout(TEST_TIMEOUT * 3);
168168

169169
test('With Changes and Config implicit from cwd', async () => {
170170
const textDocument = await workspace.openTextDocument(fileToFormatWithConfig);

extensions/positron-python/src/test/linters/lint.args.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,12 @@ suite('Linting - Arguments', () => {
142142
}
143143
test('Flake8', async () => {
144144
const linter = new Flake8(serviceContainer);
145-
const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
145+
const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
146146
await testLinter(linter, expectedArgs);
147147
});
148148
test('Pycodestyle', async () => {
149149
const linter = new Pycodestyle(serviceContainer);
150-
const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
150+
const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
151151
await testLinter(linter, expectedArgs);
152152
});
153153
test('Prospector', async () => {

extensions/positron-python/src/test/linters/lint.functional.test.ts

+8
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,11 @@ class TestFixture extends BaseTestFixture {
625625
const serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>(undefined, TypeMoq.MockBehavior.Strict);
626626
const configService = TypeMoq.Mock.ofType<IConfigurationService>(undefined, TypeMoq.MockBehavior.Strict);
627627
const processLogger = TypeMoq.Mock.ofType<IProcessLogger>(undefined, TypeMoq.MockBehavior.Strict);
628+
const componentAdapter = TypeMoq.Mock.ofType<IComponentAdapter>(undefined, TypeMoq.MockBehavior.Strict);
629+
componentAdapter
630+
.setup((c) => c.getCondaEnvironment(TypeMoq.It.isAny()))
631+
.returns(() => Promise.resolve(undefined));
632+
628633
const filesystem = new FileSystem();
629634
processLogger
630635
.setup((p) => p.logProcess(TypeMoq.It.isAnyString(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
@@ -637,6 +642,9 @@ class TestFixture extends BaseTestFixture {
637642
serviceContainer
638643
.setup((s) => s.get(TypeMoq.It.isValue(IFileSystem), TypeMoq.It.isAny()))
639644
.returns(() => filesystem);
645+
serviceContainer
646+
.setup((s) => s.get(TypeMoq.It.isValue(IComponentAdapter), TypeMoq.It.isAny()))
647+
.returns(() => componentAdapter.object);
640648

641649
const platformService = new PlatformService();
642650

0 commit comments

Comments
 (0)