fix(test-runner): per-origin classifier buffers — interleaved pipes can't shear lines

stdout and stderr are independent pipes; a chunk from one can arrive
between two halves of a line from the other. The single shared
pending-buffer glued those fragments into garbled lines: a sheared
(fail) line went uncounted (defeating the exit-0-with-failures
backstop) and a sheared terminal summary read as truncation.
Counters stay shared; line assembly is now per-stream, and both
runners tag the stream origin. Also drops the dead ChildProcess
type import left by the killProcessGroup move.
This commit is contained in:
Garry Tan
2026-08-15 16:49:38 -07:00
parent f692da35d7
commit 661b3d943f
3 changed files with 58 additions and 19 deletions
+3 -3
View File
@@ -50,7 +50,7 @@
* bun run scripts/test-paid-shards.ts --timeout 600 --jobs 2 * bun run scripts/test-paid-shards.ts --timeout 600 --jobs 2
*/ */
import { spawn, type ChildProcess } from 'node:child_process'; import { spawn } from 'node:child_process';
import * as fs from 'node:fs'; import * as fs from 'node:fs';
import * as path from 'node:path'; import * as path from 'node:path';
import { normalizeRelativePath } from './test-free-shards'; import { normalizeRelativePath } from './test-free-shards';
@@ -430,8 +430,8 @@ export async function runPaidShard(
let exitCode: number | null = null; let exitCode: number | null = null;
try { try {
const streams: Array<Promise<void>> = []; const streams: Array<Promise<void>> = [];
if (child.stdout) streams.push(forwardAndClassify(child.stdout, sink(process.stdout), classifier)); if (child.stdout) streams.push(forwardAndClassify(child.stdout, sink(process.stdout), classifier, 'stdout'));
if (child.stderr) streams.push(forwardAndClassify(child.stderr, sink(process.stderr), classifier)); if (child.stderr) streams.push(forwardAndClassify(child.stderr, sink(process.stderr), classifier, 'stderr'));
exitCode = await new Promise<number | null>((resolve, reject) => { exitCode = await new Promise<number | null>((resolve, reject) => {
child.once('error', reject); child.once('error', reject);
child.once('close', (code) => resolve(code)); child.once('close', (code) => resolve(code));
+31 -16
View File
@@ -155,25 +155,39 @@ export function parseBunTerminalSummaryLine(rawLine: string): number | null {
return match ? Number.parseInt(match[1], 10) : null; return match ? Number.parseInt(match[1], 10) : null;
} }
/** Incrementally classifies output without assuming process chunks align to lines. */ /**
* Incrementally classifies output without assuming process chunks align to
* lines. Buffers are PER ORIGIN: stdout and stderr are independent pipes, so
* a chunk from one can arrive between two halves of a line from the other.
* A single shared buffer would glue those fragments into garbled lines — a
* sheared `(fail)` line goes uncounted and a sheared terminal summary reads
* as truncation. Counters are shared; only line assembly is per-stream.
*/
export type ClassifierOrigin = 'stdout' | 'stderr';
export class BunTestOutputClassifier { export class BunTestOutputClassifier {
private readonly decoder = new StringDecoder('utf8'); private readonly decoders: Record<ClassifierOrigin, StringDecoder> = {
private pending = ''; stdout: new StringDecoder('utf8'),
stderr: new StringDecoder('utf8'),
};
private pending: Record<ClassifierOrigin, string> = { stdout: '', stderr: '' };
private failedTests = 0; private failedTests = 0;
private unhandledBetweenTests = 0; private unhandledBetweenTests = 0;
private terminalFileCounts: number[] = []; private terminalFileCounts: number[] = [];
write(chunk: Uint8Array | string): void { write(chunk: Uint8Array | string, origin: ClassifierOrigin = 'stdout'): void {
this.pending += typeof chunk === 'string' this.pending[origin] += typeof chunk === 'string'
? chunk ? chunk
: this.decoder.write(Buffer.from(chunk)); : this.decoders[origin].write(Buffer.from(chunk));
this.consumeCompleteLines(); this.consumeCompleteLines(origin);
} }
end(): BunTestOutputSummary { end(): BunTestOutputSummary {
this.pending += this.decoder.end(); for (const origin of ['stdout', 'stderr'] as const) {
if (this.pending.length > 0) this.classify(this.pending); this.pending[origin] += this.decoders[origin].end();
this.pending = ''; if (this.pending[origin].length > 0) this.classify(this.pending[origin]);
this.pending[origin] = '';
}
return this.summary(); return this.summary();
} }
@@ -185,12 +199,12 @@ export class BunTestOutputClassifier {
}; };
} }
private consumeCompleteLines(): void { private consumeCompleteLines(origin: ClassifierOrigin): void {
let newline = this.pending.indexOf('\n'); let newline = this.pending[origin].indexOf('\n');
while (newline !== -1) { while (newline !== -1) {
this.classify(this.pending.slice(0, newline)); this.classify(this.pending[origin].slice(0, newline));
this.pending = this.pending.slice(newline + 1); this.pending[origin] = this.pending[origin].slice(newline + 1);
newline = this.pending.indexOf('\n'); newline = this.pending[origin].indexOf('\n');
} }
} }
@@ -227,10 +241,11 @@ export function forwardAndClassify(
stream: NodeJS.ReadableStream, stream: NodeJS.ReadableStream,
destination: NodeJS.WriteStream, destination: NodeJS.WriteStream,
classifier: BunTestOutputClassifier, classifier: BunTestOutputClassifier,
origin: ClassifierOrigin = 'stdout',
): Promise<void> { ): Promise<void> {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
stream.on('data', (chunk: Buffer | string) => { stream.on('data', (chunk: Buffer | string) => {
classifier.write(chunk); classifier.write(chunk, origin);
destination.write(chunk); destination.write(chunk);
}); });
stream.on('end', resolve); stream.on('end', resolve);
+24
View File
@@ -58,4 +58,28 @@ describe('BunTestOutputClassifier', () => {
// passes: 1 file ran, which is what was expected // passes: 1 file ran, which is what was expected
expect(strictTestExitCode(0, summary, 1)).toBe(0); expect(strictTestExitCode(0, summary, 1)).toBe(0);
}); });
// stdout and stderr are independent pipes: a chunk from one can land
// between two halves of a line from the other. A single shared buffer
// glues the fragments into garbled lines — a sheared (fail) line goes
// uncounted (defeating the exit-0-with-failures backstop) and a sheared
// summary reads as truncation. Per-origin buffers keep each stream whole.
it('a stderr chunk arriving mid-stdout-line does not shear either line', () => {
const c = new BunTestOutputClassifier();
c.write('some stdout noise without a newline yet', 'stdout');
c.write('(fail) planted [0.10ms]\n', 'stderr');
c.write(' ...rest of the stdout line\n', 'stdout');
const summary = c.end();
expect(summary.failedTests).toBe(1);
});
it('a terminal summary split around a cross-stream chunk still counts', () => {
const c = new BunTestOutputClassifier();
c.write('Ran 4 tests acr', 'stdout');
c.write('stderr diagnostics line\n', 'stderr');
c.write('oss 2 files. [1.00s]\n', 'stdout');
const summary = c.end();
expect(summary.terminalFileCounts).toEqual([2]);
expect(strictTestExitCode(0, summary, 2)).toBe(0);
});
}); });