Skip to content

Commit 1d6133b

Browse files
committed
add std out to subprocess to stop overflow (microsoft#21758)
Displays output to the user and ensures the subprocess doesn't run into buffer overflow. Bug was introduced with the switch to the execObservable in microsoft#21667. Only occurs for large repos where output from pytest was large enough to reach buffer overflow.
1 parent edaa216 commit 1d6133b

File tree

6 files changed

+43
-6
lines changed

6 files changed

+43
-6
lines changed

pythonFiles/tests/pytestadapter/test_execution.py

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
# Copyright (c) Microsoft Corporation. All rights reserved.
22
# Licensed under the MIT License.
3-
import json
43
import os
54
import shutil
65

src/client/testing/testController/common/server.ts

+10-1
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,16 @@ export class PythonTestServer implements ITestServer, Disposable {
209209
runInstance?.token.onCancellationRequested(() => {
210210
result?.proc?.kill();
211211
});
212-
result?.proc?.on('close', () => {
212+
213+
// Take all output from the subprocess and add it to the test output channel. This will be the pytest output.
214+
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
215+
result?.proc?.stdout?.on('data', (data) => {
216+
spawnOptions?.outputChannel?.append(data);
217+
});
218+
result?.proc?.stderr?.on('data', (data) => {
219+
spawnOptions?.outputChannel?.append(data);
220+
});
221+
result?.proc?.on('exit', () => {
213222
traceLog('Exec server closed.', uuid);
214223
deferred.resolve({ stdout: '', stderr: '' });
215224
callback?.();

src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts

+9-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,15 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
8686
const execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(pytestArgs);
8787
const result = execService?.execObservable(execArgs, spawnOptions);
8888

89-
result?.proc?.on('close', () => {
89+
// Take all output from the subprocess and add it to the test output channel. This will be the pytest output.
90+
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
91+
result?.proc?.stdout?.on('data', (data) => {
92+
spawnOptions.outputChannel?.append(data);
93+
});
94+
result?.proc?.stderr?.on('data', (data) => {
95+
spawnOptions.outputChannel?.append(data);
96+
});
97+
result?.proc?.on('exit', () => {
9098
deferredExec.resolve({ stdout: '', stderr: '' });
9199
deferred.resolve();
92100
});

src/client/testing/testController/pytest/pytestExecutionAdapter.ts

+9
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,15 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
166166
result?.proc?.kill();
167167
});
168168

169+
// Take all output from the subprocess and add it to the test output channel. This will be the pytest output.
170+
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
171+
result?.proc?.stdout?.on('data', (data) => {
172+
this.outputChannel?.append(data);
173+
});
174+
result?.proc?.stderr?.on('data', (data) => {
175+
this.outputChannel?.append(data);
176+
});
177+
169178
result?.proc?.on('close', () => {
170179
deferredExec.resolve({ stdout: '', stderr: '' });
171180
deferred.resolve();

src/test/mocks/helper.ts

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { Readable } from 'stream';
5+
6+
export class FakeReadableStream extends Readable {
7+
_read(_size: unknown): void | null {
8+
// custom reading logic here
9+
this.push(null); // end the stream
10+
}
11+
}

src/test/mocks/mockChildProcess.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
/* eslint-disable @typescript-eslint/no-unused-vars */
22
/* eslint-disable @typescript-eslint/no-explicit-any */
33
import { Serializable, SendHandle, MessageOptions } from 'child_process';
4-
import { Writable, Readable, Pipe } from 'stream';
54
import { EventEmitter } from 'node:events';
5+
import { Writable, Readable, Pipe } from 'stream';
6+
import { FakeReadableStream } from './helper';
67

78
export class MockChildProcess extends EventEmitter {
89
constructor(spawnfile: string, spawnargs: string[]) {
910
super();
1011
this.spawnfile = spawnfile;
1112
this.spawnargs = spawnargs;
1213
this.stdin = new Writable();
13-
this.stdout = new Readable();
14-
this.stderr = null;
14+
this.stdout = new FakeReadableStream();
15+
this.stderr = new FakeReadableStream();
1516
this.channel = null;
1617
this.stdio = [this.stdin, this.stdout, this.stdout, this.stderr, null];
1718
this.killed = false;

0 commit comments

Comments
 (0)