Skip to content

Commit 9bc542e

Browse files
author
Kartik Raj
authored
Handle scenarios where validating interpreters via conda run emits more than just the validation script output (microsoft#18176)
* Handle scenarios where running interpreterInfo.py via conda run emits more than just the JSON file * Update tests * Even if there is a stderr, attempt parsing the output * Fix compile error * Fix black formatting * Fix tests
1 parent 27f3194 commit 9bc542e

8 files changed

Lines changed: 95 additions & 23 deletions

File tree

‎pythonFiles/interpreterInfo.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,7 @@
1010
obj["sysVersion"] = sys.version
1111
obj["is64Bit"] = sys.maxsize > 2 ** 32
1212

13+
# Printing out markers for our JSON to make it more resilient to pull the output.
14+
print(">>>JSON")
1315
print(json.dumps(obj))
16+
print("<<<JSON")

‎src/client/common/process/internal/scripts/index.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,19 @@ export type InterpreterInfoJson = {
4141
is64Bit: boolean;
4242
};
4343

44-
export function interpreterInfo(): [string[], (out: string) => InterpreterInfoJson] {
44+
export function interpreterInfo(): [string[], (out: string) => InterpreterInfoJson | undefined] {
4545
const script = path.join(SCRIPTS_DIR, 'interpreterInfo.py');
4646
const args = [script];
4747

48-
function parse(out: string): InterpreterInfoJson {
49-
let json: InterpreterInfoJson;
48+
function parse(out: string): InterpreterInfoJson | undefined {
49+
const regex = />>>JSON([\s\S]*)<<<JSON/;
50+
const match = out.match(regex);
51+
const filteredOut = match !== null && match.length >= 2 ? match[1].trim() : '';
52+
let json: InterpreterInfoJson | undefined;
5053
try {
51-
json = JSON.parse(out);
54+
json = JSON.parse(filteredOut);
5255
} catch (ex) {
53-
throw Error(`python ${args} returned bad JSON (${out}) (${ex})`);
56+
throw Error(`python ${args} returned bad JSON (${out}) (${filteredOut}) (${ex})`);
5457
}
5558
return json;
5659
}

‎src/client/pythonEnvironments/base/info/interpreter.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,14 @@ export async function getInterpreterInfo(python: PythonExecInfo): Promise<Interp
8383
// https://github.com/microsoft/vscode-python/issues/7760
8484
const result = await shellExecute(quoted, { timeout: 15000 });
8585
if (result.stderr) {
86-
traceError(`Failed to parse interpreter information for ${argv} stderr: ${result.stderr}`);
87-
return undefined;
86+
traceError(
87+
`Stderr when executing script with ${argv} stderr: ${result.stderr}, still attempting to parse output`,
88+
);
8889
}
8990
const json = parse(result.stdout);
91+
if (!json) {
92+
return undefined;
93+
}
9094
traceInfo(`Found interpreter for ${argv}`);
9195
return extractInterpreterInfo(python.pythonExecutable, json);
9296
}

‎src/client/pythonEnvironments/info/interpreter.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,13 @@ export async function getInterpreterInfo(
8686
if (logger) {
8787
logger.error(`Failed to parse interpreter information for ${argv} stderr: ${result.stderr}`);
8888
}
89-
return undefined;
9089
}
9190
const json = parse(result.stdout);
9291
if (logger) {
9392
logger.info(`Found interpreter for ${argv}`);
9493
}
94+
if (!json) {
95+
return undefined;
96+
}
9597
return extractInterpreterInfo(python.pythonExecutable, json);
9698
}

‎src/test/common/process/pythonEnvironment.unit.test.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,13 @@ suite('PythonEnvironment', () => {
3636

3737
processService
3838
.setup((p) => p.shellExec(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
39-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
39+
.returns(() =>
40+
Promise.resolve({
41+
stdout: `>>>JSON
42+
${JSON.stringify(json)}
43+
<<<JSON`,
44+
}),
45+
);
4046
const env = createPythonEnv(pythonPath, processService.object, fileSystem.object);
4147

4248
const result = await env.getInterpreterInformation();
@@ -61,7 +67,13 @@ suite('PythonEnvironment', () => {
6167

6268
processService
6369
.setup((p) => p.shellExec(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
64-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
70+
.returns(() =>
71+
Promise.resolve({
72+
stdout: `>>>JSON
73+
${JSON.stringify(json)}
74+
<<<JSON`,
75+
}),
76+
);
6577
const env = createPythonEnv(pythonPath, processService.object, fileSystem.object);
6678

6779
const result = await env.getInterpreterInformation();
@@ -89,7 +101,13 @@ suite('PythonEnvironment', () => {
89101

90102
processService
91103
.setup((p) => p.shellExec(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
92-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
104+
.returns(() =>
105+
Promise.resolve({
106+
stdout: `>>>JSON
107+
${JSON.stringify(json)}
108+
<<<JSON`,
109+
}),
110+
);
93111
const env = createPythonEnv(pythonPath, processService.object, fileSystem.object);
94112

95113
const result = await env.getInterpreterInformation();
@@ -117,7 +135,13 @@ suite('PythonEnvironment', () => {
117135

118136
processService
119137
.setup((p) => p.shellExec(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
120-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
138+
.returns(() =>
139+
Promise.resolve({
140+
stdout: `>>>JSON
141+
${JSON.stringify(json)}
142+
<<<JSON`,
143+
}),
144+
);
121145
const env = createPythonEnv(pythonPath, processService.object, fileSystem.object);
122146

123147
const result = await env.getInterpreterInformation();

‎src/test/pythonEnvironments/base/info/environmentInfoService.functional.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ suite('Environment Info Service', () => {
4949
new Promise<ExecutionResult<string>>((resolve) => {
5050
resolve({
5151
stdout:
52-
'{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}',
52+
'SomeGarbage>>>JSON\n{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}\n<<<JSON_SomeGarbage',
53+
stderr: 'Some std error', // This should be ignored.
5354
});
5455
}),
5556
);

‎src/test/pythonEnvironments/base/locators/composite/envsResolver.unit.test.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ suite('Python envs locator - Environments Resolver', () => {
9696
new Promise<ExecutionResult<string>>((resolve) => {
9797
resolve({
9898
stdout:
99-
'{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}',
99+
'SomeGarbage>>>JSON\n{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}\n<<<JSON_SomeGarbage',
100100
});
101101
}),
102102
);
@@ -160,7 +160,6 @@ suite('Python envs locator - Environments Resolver', () => {
160160
stubShellExec.returns(
161161
new Promise<ExecutionResult<string>>((resolve) => {
162162
resolve({
163-
stderr: 'Kaboom',
164163
stdout: '',
165164
});
166165
}),
@@ -251,7 +250,7 @@ suite('Python envs locator - Environments Resolver', () => {
251250
new Promise<ExecutionResult<string>>((resolve) => {
252251
resolve({
253252
stdout:
254-
'{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}',
253+
'SomeGarbage>>>JSON\n{"versionInfo": [3, 8, 3, "final", 0], "sysPrefix": "path", "sysVersion": "3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]", "is64Bit": true}\n<<<JSON_SomeGarbage',
255254
});
256255
}),
257256
);
@@ -295,7 +294,7 @@ suite('Python envs locator - Environments Resolver', () => {
295294
assert.deepEqual(expected, undefined);
296295
});
297296

298-
test('If fetching interpreter info fails with stderr, return undefined', async () => {
297+
test('If parsing interpreter info fails, return undefined', async () => {
299298
stubShellExec.returns(
300299
new Promise<ExecutionResult<string>>((resolve) => {
301300
resolve({

‎src/test/pythonEnvironments/info/interpreter.unit.test.ts‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,13 @@ suite('getInterpreterInfo()', () => {
4444
deps
4545
// Checking the args is the key point of this test.
4646
.setup((d) => d.shellExec(cmd, 15000))
47-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
47+
.returns(() =>
48+
Promise.resolve({
49+
stdout: `>>>JSON
50+
${JSON.stringify(json)}
51+
<<<JSON`,
52+
}),
53+
);
4854
const shellExec = async (c: string, t: number) => deps.object.shellExec(c, t);
4955

5056
await getInterpreterInfo(python, shellExec);
@@ -64,7 +70,13 @@ suite('getInterpreterInfo()', () => {
6470
deps
6571
// Checking the args is the key point of this test.
6672
.setup((d) => d.shellExec(cmd, 15000))
67-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
73+
.returns(() =>
74+
Promise.resolve({
75+
stdout: `>>>JSON
76+
${JSON.stringify(json)}
77+
<<<JSON`,
78+
}),
79+
);
6880
const shellExec = async (c: string, t: number) => deps.object.shellExec(c, t);
6981

7082
await getInterpreterInfo(_python, shellExec);
@@ -84,7 +96,13 @@ suite('getInterpreterInfo()', () => {
8496
deps
8597
// Checking the args is the key point of this test.
8698
.setup((d) => d.shellExec(cmd, 15000))
87-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
99+
.returns(() =>
100+
Promise.resolve({
101+
stdout: `>>>JSON
102+
${JSON.stringify(json)}
103+
<<<JSON`,
104+
}),
105+
);
88106
const shellExec = async (c: string, t: number) => deps.object.shellExec(c, t);
89107

90108
await getInterpreterInfo(_python, shellExec);
@@ -109,7 +127,13 @@ suite('getInterpreterInfo()', () => {
109127
deps
110128
// We check the args in other tests.
111129
.setup((d) => d.shellExec(TypeMoqIt.isAny(), TypeMoqIt.isAny()))
112-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
130+
.returns(() =>
131+
Promise.resolve({
132+
stdout: `>>>JSON
133+
${JSON.stringify(json)}
134+
<<<JSON`,
135+
}),
136+
);
113137
const shellExec = async (c: string, t: number) => deps.object.shellExec(c, t);
114138

115139
const result = await getInterpreterInfo(python, shellExec);
@@ -135,7 +159,13 @@ suite('getInterpreterInfo()', () => {
135159
deps
136160
// We check the args in other tests.
137161
.setup((d) => d.shellExec(TypeMoqIt.isAny(), TypeMoqIt.isAny()))
138-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
162+
.returns(() =>
163+
Promise.resolve({
164+
stdout: `>>>JSON
165+
${JSON.stringify(json)}
166+
<<<JSON`,
167+
}),
168+
);
139169
const shellExec = async (c: string, t: number) => deps.object.shellExec(c, t);
140170

141171
const result = await getInterpreterInfo(python, shellExec);
@@ -161,7 +191,13 @@ suite('getInterpreterInfo()', () => {
161191
deps
162192
// We check the args in other tests.
163193
.setup((d) => d.shellExec(TypeMoqIt.isAny(), TypeMoqIt.isAny()))
164-
.returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
194+
.returns(() =>
195+
Promise.resolve({
196+
stdout: `>>>JSON
197+
${JSON.stringify(json)}
198+
<<<JSON`,
199+
}),
200+
);
165201
const shellExec = async (c: string, t: number) => deps.object.shellExec(c, t);
166202

167203
const result = await getInterpreterInfo(python, shellExec);

0 commit comments

Comments
 (0)