Skip to content

Commit 7580f32

Browse files
author
Kartik Raj
committed
Add more tests
1 parent 4c742ce commit 7580f32

File tree

5 files changed

+42
-39
lines changed

5 files changed

+42
-39
lines changed

src/client/common/process/proc.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { IDisposable } from '../types';
77
import { EnvironmentVariables } from '../variables/types';
88
import { execObservable, killPid, plainExec, shellExec } from './rawProcessApis';
99
import { ExecutionResult, IProcessService, ObservableExecutionResult, ShellOptions, SpawnOptions } from './types';
10+
import { workerPlainExec, workerShellExec } from './worker/rawProcessApiWrapper';
1011

1112
export class ProcessService extends EventEmitter implements IProcessService {
1213
private processesToKill = new Set<IDisposable>();
@@ -47,14 +48,20 @@ export class ProcessService extends EventEmitter implements IProcessService {
4748
}
4849

4950
public exec(file: string, args: string[], options: SpawnOptions = {}): Promise<ExecutionResult<string>> {
51+
this.emit('exec', file, args, options);
52+
if (options.useWorker) {
53+
return workerPlainExec(file, args, options);
54+
}
5055
const execOptions = { ...options, doNotLog: true };
5156
const promise = plainExec(file, args, execOptions, this.env, this.processesToKill);
52-
this.emit('exec', file, args, options);
5357
return promise;
5458
}
5559

5660
public shellExec(command: string, options: ShellOptions = {}): Promise<ExecutionResult<string>> {
5761
this.emit('exec', command, undefined, options);
62+
if (options.useWorker) {
63+
return workerShellExec(command, options);
64+
}
5865
const disposables = new Set<IDisposable>();
5966
const shellOptions = { ...options, doNotLog: true };
6067
return shellExec(command, shellOptions, this.env, disposables).finally(() => {

src/client/common/process/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ export type SpawnOptions = ChildProcessSpawnOptions & {
2626
extraVariables?: NodeJS.ProcessEnv;
2727
outputChannel?: OutputChannel;
2828
stdinStr?: string;
29+
useWorker?: boolean;
2930
};
3031

31-
export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean };
32+
export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean; useWorker?: boolean };
3233

3334
export type ExecutionResult<T extends string | Buffer> = {
3435
stdout: T;

src/client/common/process/worker/rawProcessApiWrapper.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,10 @@
33

44
import { SpawnOptions } from 'child_process';
55
import * as path from 'path';
6-
import { IProcessLogger } from '../types';
76
import { executeWorkerFile } from './main';
87
import { ExecutionResult, ShellOptions } from './types';
98

10-
export function workerShellExec(
11-
command: string,
12-
options: ShellOptions,
13-
processLogger: IProcessLogger,
14-
): Promise<ExecutionResult<string>> {
15-
processLogger.logProcess(command, undefined, options);
9+
export function workerShellExec(command: string, options: ShellOptions): Promise<ExecutionResult<string>> {
1610
return executeWorkerFile(path.join(__dirname, 'shellExecWorker.js'), {
1711
command,
1812
options,
@@ -23,9 +17,7 @@ export function workerPlainExec(
2317
file: string,
2418
args: string[],
2519
options: SpawnOptions = {},
26-
processLogger: IProcessLogger,
2720
): Promise<ExecutionResult<string>> {
28-
processLogger.logProcess(file, args, options);
2921
return executeWorkerFile(path.join(__dirname, 'plainExecWorker.js'), {
3022
file,
3123
args,

src/client/pythonEnvironments/common/externalDependencies.ts

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,13 @@ import * as fsapi from 'fs-extra';
55
import * as path from 'path';
66
import * as vscode from 'vscode';
77
import { IWorkspaceService } from '../../common/application/types';
8-
import {
9-
ExecutionResult,
10-
IProcessLogger,
11-
IProcessServiceFactory,
12-
ShellOptions,
13-
SpawnOptions,
14-
} from '../../common/process/types';
8+
import { ExecutionResult, IProcessServiceFactory, ShellOptions, SpawnOptions } from '../../common/process/types';
159
import { IDisposable, IConfigurationService, IExperimentService } from '../../common/types';
1610
import { chain, iterable } from '../../common/utils/async';
1711
import { getOSType, OSType } from '../../common/utils/platform';
1812
import { IServiceContainer } from '../../ioc/types';
1913
import { traceError, traceVerbose } from '../../logging';
2014
import { DiscoveryUsingWorkers } from '../../common/experiments/groups';
21-
import { workerPlainExec, workerShellExec } from '../../common/process/worker/rawProcessApiWrapper';
22-
import { IEnvironmentVariablesProvider } from '../../common/variables/types';
2315

2416
let internalServiceContainer: IServiceContainer;
2517
export function initializeExternalDependencies(serviceContainer: IServiceContainer): void {
@@ -29,32 +21,21 @@ export function initializeExternalDependencies(serviceContainer: IServiceContain
2921
// processes
3022

3123
export async function shellExecute(command: string, options: ShellOptions = {}): Promise<ExecutionResult<string>> {
32-
if (!inExperiment(DiscoveryUsingWorkers.experiment)) {
33-
const service = await internalServiceContainer.get<IProcessServiceFactory>(IProcessServiceFactory).create();
34-
return service.shellExec(command, options);
35-
}
36-
const logger = internalServiceContainer.get<IProcessLogger>(IProcessLogger);
37-
const envVarsService = internalServiceContainer.get<IEnvironmentVariablesProvider>(IEnvironmentVariablesProvider);
38-
const envs = await envVarsService.getEnvironmentVariables();
39-
options.env = { ...options.env, ...envs };
40-
return workerShellExec(command, options, logger);
24+
const useWorker = inExperiment(DiscoveryUsingWorkers.experiment);
25+
const service = await internalServiceContainer.get<IProcessServiceFactory>(IProcessServiceFactory).create();
26+
options = { ...options, useWorker };
27+
return service.shellExec(command, options);
4128
}
4229

4330
export async function exec(
4431
file: string,
4532
args: string[],
4633
options: SpawnOptions = {},
47-
useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment),
34+
useWorker = inExperiment(DiscoveryUsingWorkers.experiment),
4835
): Promise<ExecutionResult<string>> {
49-
if (!useWorkerThreads) {
50-
const service = await internalServiceContainer.get<IProcessServiceFactory>(IProcessServiceFactory).create();
51-
return service.exec(file, args, options);
52-
}
53-
const logger = internalServiceContainer.get<IProcessLogger>(IProcessLogger);
54-
const envVarsService = internalServiceContainer.get<IEnvironmentVariablesProvider>(IEnvironmentVariablesProvider);
55-
const envs = await envVarsService.getEnvironmentVariables();
56-
options.env = { ...options.env, ...envs };
57-
return workerPlainExec(file, args, options, logger);
36+
const service = await internalServiceContainer.get<IProcessServiceFactory>(IProcessServiceFactory).create();
37+
options = { ...options, useWorker };
38+
return service.exec(file, args, options);
5839
}
5940

6041
export function inExperiment(experimentName: string): boolean {

src/test/common/process/proc.exec.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@ suite('ProcessService Observable', () => {
3434
expect(result.stderr).to.equal(undefined, 'stderr not undefined');
3535
});
3636

37+
test('When using worker threads, exec should output print statements', async () => {
38+
const procService = new ProcessService();
39+
const printOutput = '1234';
40+
const result = await procService.exec(pythonPath, ['-c', `print("${printOutput}")`], { useWorker: true });
41+
42+
expect(result).not.to.be.an('undefined', 'result is undefined');
43+
expect(result.stdout.trim()).to.be.equal(printOutput, 'Invalid output');
44+
expect(result.stderr).to.equal(undefined, 'stderr not undefined');
45+
});
46+
3747
test('exec should output print unicode characters', async function () {
3848
// This test has not been working for many months in Python 2.7 under
3949
// Windows. Tracked by #2546. (unicode under Py2.7 is tough!)
@@ -241,6 +251,18 @@ suite('ProcessService Observable', () => {
241251
expect(result.stderr).to.equal(undefined, 'stderr not empty');
242252
expect(result.stdout.trim()).to.be.equal(printOutput, 'Invalid output');
243253
});
254+
test('When using worker threads, shellExec should be able to run python and filter output using conda related markers', async () => {
255+
const procService = new ProcessService();
256+
const printOutput = '1234';
257+
const result = await procService.shellExec(
258+
`"${pythonPath}" -c "print('>>>PYTHON-EXEC-OUTPUT');print('${printOutput}');print('<<<PYTHON-EXEC-OUTPUT')"`,
259+
{ useWorker: true },
260+
);
261+
262+
expect(result).not.to.be.an('undefined', 'result is undefined');
263+
expect(result.stderr).to.equal(undefined, 'stderr not empty');
264+
expect(result.stdout.trim()).to.be.equal(printOutput, 'Invalid output');
265+
});
244266
test('shellExec should fail on invalid command', async () => {
245267
const procService = new ProcessService();
246268
const result = procService.shellExec('invalid command');

0 commit comments

Comments
 (0)