Skip to content

Commit

Permalink
add std out to subprocess to stop overflow (#21758)
Browse files Browse the repository at this point in the history
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 #21667.
Only occurs for large repos where output from pytest was large enough to
reach buffer overflow.
  • Loading branch information
eleanorjboyd authored Aug 11, 2023
1 parent b299ec9 commit 461fb91
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 6 deletions.
1 change: 0 additions & 1 deletion pythonFiles/tests/pytestadapter/test_execution.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.
import json
import os
import shutil

Expand Down
11 changes: 10 additions & 1 deletion src/client/testing/testController/common/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,16 @@ export class PythonTestServer implements ITestServer, Disposable {
runInstance?.token.onCancellationRequested(() => {
result?.proc?.kill();
});
result?.proc?.on('close', () => {

// Take all output from the subprocess and add it to the test output channel. This will be the pytest output.
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
result?.proc?.stdout?.on('data', (data) => {
spawnOptions?.outputChannel?.append(data);
});
result?.proc?.stderr?.on('data', (data) => {
spawnOptions?.outputChannel?.append(data);
});
result?.proc?.on('exit', () => {
traceLog('Exec server closed.', uuid);
deferred.resolve({ stdout: '', stderr: '' });
callback?.();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,15 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
const execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(pytestArgs);
const result = execService?.execObservable(execArgs, spawnOptions);

result?.proc?.on('close', () => {
// Take all output from the subprocess and add it to the test output channel. This will be the pytest output.
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
result?.proc?.stdout?.on('data', (data) => {
spawnOptions.outputChannel?.append(data);
});
result?.proc?.stderr?.on('data', (data) => {
spawnOptions.outputChannel?.append(data);
});
result?.proc?.on('exit', () => {
deferredExec.resolve({ stdout: '', stderr: '' });
deferred.resolve();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
result?.proc?.kill();
});

// Take all output from the subprocess and add it to the test output channel. This will be the pytest output.
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
result?.proc?.stdout?.on('data', (data) => {
this.outputChannel?.append(data);
});
result?.proc?.stderr?.on('data', (data) => {
this.outputChannel?.append(data);
});

result?.proc?.on('close', () => {
deferredExec.resolve({ stdout: '', stderr: '' });
deferred.resolve();
Expand Down
11 changes: 11 additions & 0 deletions src/test/mocks/helper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { Readable } from 'stream';

export class FakeReadableStream extends Readable {
_read(_size: unknown): void | null {
// custom reading logic here
this.push(null); // end the stream
}
}
7 changes: 4 additions & 3 deletions src/test/mocks/mockChildProcess.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
/* eslint-disable @typescript-eslint/no-unused-vars */
/* eslint-disable @typescript-eslint/no-explicit-any */
import { Serializable, SendHandle, MessageOptions } from 'child_process';
import { Writable, Readable, Pipe } from 'stream';
import { EventEmitter } from 'node:events';
import { Writable, Readable, Pipe } from 'stream';
import { FakeReadableStream } from './helper';

export class MockChildProcess extends EventEmitter {
constructor(spawnfile: string, spawnargs: string[]) {
super();
this.spawnfile = spawnfile;
this.spawnargs = spawnargs;
this.stdin = new Writable();
this.stdout = new Readable();
this.stderr = null;
this.stdout = new FakeReadableStream();
this.stderr = new FakeReadableStream();
this.channel = null;
this.stdio = [this.stdin, this.stdout, this.stdout, this.stderr, null];
this.killed = false;
Expand Down

0 comments on commit 461fb91

Please sign in to comment.