Skip to content

Commit 22c5211

Browse files
authored
Fix flaky tests related to auto selection of virtual environments (microsoft#3728)
For microsoft#2339
1 parent 77595a1 commit 22c5211

File tree

3 files changed

+24
-29
lines changed

3 files changed

+24
-29
lines changed

news/3 Code Health/2339.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix flaky tests related to auto selection of virtual environments.

src/client/common/process/proc.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,13 @@ export class ProcessService implements IProcessService {
167167
});
168168
}
169169

170-
private getDefaultOptions<T extends (ShellOptions | SpawnOptions)>(options: T) : T {
170+
private getDefaultOptions<T extends (ShellOptions | SpawnOptions)>(options: T): T {
171171
const defaultOptions = { ...options };
172172
const execOptions = defaultOptions as SpawnOptions;
173-
if (execOptions)
174-
{
173+
if (execOptions) {
175174
const encoding = execOptions.encoding = typeof execOptions.encoding === 'string' && execOptions.encoding.length > 0 ? execOptions.encoding : DEFAULT_ENCODING;
176175
delete execOptions.encoding;
177176
execOptions.encoding = encoding;
178-
execOptions.token = execOptions.token;
179177
}
180178
if (!defaultOptions.env || Object.keys(defaultOptions.env).length === 0) {
181179
const env = this.env ? this.env : process.env;

src/test/interpreters/locators/workspaceVirtualEnvService.test.ts

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,50 +5,47 @@
55

66
// tslint:disable:no-any max-classes-per-file max-func-body-length no-invalid-this
77
import { expect } from 'chai';
8-
import { spawn } from 'child_process';
8+
import { exec } from 'child_process';
99
import * as path from 'path';
1010
import { Uri } from 'vscode';
11-
import { sleep } from '../../../client/common/utils/async';
11+
import '../../../client/common/extensions';
1212
import { IInterpreterLocatorService, WORKSPACE_VIRTUAL_ENV_SERVICE } from '../../../client/interpreter/contracts';
1313
import { IServiceContainer } from '../../../client/ioc/types';
14-
import { deleteFiles, isPythonVersionInProcess, PYTHON_PATH, rootWorkspaceUri } from '../../common';
14+
import { deleteFiles, isPythonVersionInProcess, PYTHON_PATH, rootWorkspaceUri, waitForCondition } from '../../common';
1515
import { IS_MULTI_ROOT_TEST } from '../../constants';
1616
import { initialize, multirootPath } from '../../initialize';
1717

18-
const timeoutSecs = 120;
18+
const timeoutMs = 60_000;
1919
suite('Interpreters - Workspace VirtualEnv Service', function () {
20-
this.timeout(timeoutSecs * 1000);
20+
this.timeout(timeoutMs);
2121
this.retries(1);
2222

2323
let locator: IInterpreterLocatorService;
24-
const workspaceUri = IS_MULTI_ROOT_TEST ? Uri.file(path.join(multirootPath, 'workspace3')) : rootWorkspaceUri;
24+
const workspaceUri = IS_MULTI_ROOT_TEST ? Uri.file(path.join(multirootPath, 'workspace3')) : rootWorkspaceUri!;
2525
const workspace4 = Uri.file(path.join(multirootPath, 'workspace4'));
2626
const venvPrefix = '.venv';
2727
let serviceContainer: IServiceContainer;
2828

2929
async function waitForInterpreterToBeDetected(envNameToLookFor: string) {
30-
for (let i = 0; i < timeoutSecs; i += 1) {
30+
const predicate = async () => {
3131
const items = await locator.getInterpreters(workspaceUri);
32-
if (items.some(item => item.envName === envNameToLookFor && !item.cachedEntry)) {
33-
return;
34-
}
35-
await sleep(500);
36-
}
37-
throw new Error(`${envNameToLookFor}, Environment not detected in the workspace ${workspaceUri.fsPath}`);
32+
return items.some(item => item.envName === envNameToLookFor && !item.cachedEntry);
33+
};
34+
await waitForCondition(predicate, timeoutMs, `${envNameToLookFor}, Environment not detected in the workspace ${workspaceUri.fsPath}`);
3835
}
3936
async function createVirtualEnvironment(envSuffix: string) {
4037
// Ensure env is random to avoid conflicts in tests (currupting test data).
4138
const envName = `${venvPrefix}${envSuffix}${new Date().getTime().toString()}`;
4239
return new Promise<string>((resolve, reject) => {
43-
const proc = spawn(PYTHON_PATH, ['-m', 'venv', envName], { cwd: workspaceUri.fsPath });
44-
let stdErr = '';
45-
proc.stderr.on('data', data => stdErr += data.toString());
46-
proc.on('exit', () => {
47-
if (stdErr.length === 0) {
48-
return resolve(envName);
40+
exec(`${PYTHON_PATH.fileToCommandArgument()} -m venv ${envName}`, { cwd: workspaceUri.fsPath }, (ex, _, stderr) => {
41+
if (ex) {
42+
return reject(ex);
43+
}
44+
if (stderr && stderr.length > 0) {
45+
reject(new Error(`Failed to create Env ${envName}, ${PYTHON_PATH}, Error: ${stderr}`));
46+
} else {
47+
resolve(envName);
4948
}
50-
const err = new Error(`Failed to create Env ${envName}, ${PYTHON_PATH}, Error: ${stdErr.toString()}`);
51-
reject(err);
5249
});
5350
});
5451
}
@@ -59,24 +56,23 @@ suite('Interpreters - Workspace VirtualEnv Service', function () {
5956
}
6057
serviceContainer = (await initialize()).serviceContainer;
6158
locator = serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, WORKSPACE_VIRTUAL_ENV_SERVICE);
59+
// This test is required, we need to wait for interpreter listing completes,
60+
// before proceeding with other tests.
61+
await locator.getInterpreters(workspaceUri);
6262

6363
await deleteFiles(path.join(workspaceUri.fsPath, `${venvPrefix}*`));
64-
// https://github.com/Microsoft/vscode-python/issues/3239
65-
this.skip();
6664
});
6765

6866
suiteTeardown(() => deleteFiles(path.join(workspaceUri.fsPath, `${venvPrefix}*`)));
6967
teardown(() => deleteFiles(path.join(workspaceUri.fsPath, `${venvPrefix}*`)));
7068

7169
test('Detect Virtual Environment', async () => {
7270
const envName = await createVirtualEnvironment('one');
73-
7471
await waitForInterpreterToBeDetected(envName);
7572
});
7673

7774
test('Detect a new Virtual Environment', async () => {
7875
const env1 = await createVirtualEnvironment('first');
79-
8076
await waitForInterpreterToBeDetected(env1);
8177

8278
// Ensure second environment in our workspace folder is detected when created.

0 commit comments

Comments
 (0)