Skip to content

Commit 6083b1d

Browse files
thymikeethymikee
authored
test: route every stdout/stderr write swap through stream-capture (#3473)
Makes stream-capture.ts the one place that changes process.stdout.write or process.stderr.write. Migrate the remaining hand-rolled swaps: the daemon-http-fixture captureStderr (its restore assigned a bound copy, leaving an own-property shadow on the real stream for the rest of the file) with its callers in daemon-client-lifecycle and daemon-client-newer-daemon, the stdout captures in generic and screenshot, the scripts lanes (check-affected, coverage-changed, coverage-crap, fuzz worker, help-conformance sample producers), and the first-round consumers updated to the slimmed surface. The seven vi.spyOn(process.stdout|stderr, 'write') sites move too: a spyOn write fake has the same leak shape as the assignment swaps. Shrink the helper surface per review: beginStdoutCapture and beginStderrCapture stay the primitives; withCapturedStderr gains an interactive option and absorbs withInteractiveStderr/Async; withStreamTtyAsync (no caller outside its own test) is gone; the fuzz worker's full-life suppression becomes the discard option. After this, no test changes process.stdout/stderr.write by assignment or spy. Covers #3421
1 parent d97c9bf commit 6083b1d

32 files changed

Lines changed: 408 additions & 570 deletions

‎scripts/__tests__/help-conformance-sample-producers.ts‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { CapturedSample } from '../help-conformance-sample-outputs.mjs';
2+
import { withCapturedStderrAsync } from '../../src/__tests__/test-utils/stream-capture.ts';
23
import {
34
AMBIGUOUS_MATCH_SAMPLE,
45
APP_NOT_INSTALLED_SAMPLE,
@@ -66,18 +67,10 @@ const formatFill = (result: Record<string, unknown>) =>
6667

6768
/** The `Error (CODE): …` + `Hint: …` text printHumanError writes to stderr. */
6869
async function renderHumanError(error: AppError): Promise<string> {
69-
const lines: string[] = [];
70-
const originalWrite = process.stderr.write;
71-
process.stderr.write = ((chunk: string | Uint8Array) => {
72-
lines.push(String(chunk));
73-
return true;
74-
}) as typeof process.stderr.write;
75-
try {
70+
const stderr = await withCapturedStderrAsync(async () => {
7671
await printHumanError(normalizeError(error));
77-
} finally {
78-
process.stderr.write = originalWrite;
79-
}
80-
return lines.join('').trimEnd();
72+
});
73+
return stderr.trimEnd();
8174
}
8275

8376
type ErrorResponse = {

‎scripts/check-affected/run.test.ts‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import path from 'node:path';
1010
import { test } from 'node:test';
1111
import { runCmdSync } from '@agent-device/host-kit/command';
1212
import { STALE_NODE_MODULES_MESSAGE } from './lockfile-install-sync.ts';
13+
import { beginStderrCapture } from '../../src/__tests__/test-utils/stream-capture.ts';
1314
import { CHECK_CATALOG } from './checks.ts';
1415
import { selectChecks } from './model.ts';
1516
import { type CommandExecutor, readChangedFiles, runChecks } from './run.ts';
@@ -320,12 +321,7 @@ test('runChecks proceeds normally when the install is in sync', async () => {
320321
});
321322

322323
test('runChecks names the real cause on stderr instead of surfacing as an unrelated failure', async () => {
323-
const stderrChunks: string[] = [];
324-
const originalWrite = process.stderr.write.bind(process.stderr);
325-
process.stderr.write = ((chunk: string | Uint8Array) => {
326-
stderrChunks.push(chunk.toString());
327-
return true;
328-
}) as typeof process.stderr.write;
324+
const stderrCapture = beginStderrCapture();
329325

330326
try {
331327
const plan = selectChecks({
@@ -339,14 +335,14 @@ test('runChecks names the real cause on stderr instead of surfacing as an unrela
339335
});
340336
assert.equal(code, 1);
341337
assert.ok(
342-
stderrChunks.some((chunk) => chunk.includes(STALE_NODE_MODULES_MESSAGE)),
343-
`expected stderr to name the stale-install cause, got: ${stderrChunks.join('')}`,
338+
stderrCapture.read().includes(STALE_NODE_MODULES_MESSAGE),
339+
`expected stderr to name the stale-install cause, got: ${stderrCapture.read()}`,
344340
);
345-
const stderr = stderrChunks.join('');
341+
const stderr = stderrCapture.read();
346342
assert.match(stderr, /Worktree: \.\n/);
347343
assert.match(stderr, /pnpm install --frozen-lockfile/);
348344
assert.doesNotMatch(stderr, /agent-device doctor/);
349345
} finally {
350-
process.stderr.write = originalWrite;
346+
stderrCapture.restore();
351347
}
352348
});

‎scripts/coverage-changed/run.test.ts‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import os from 'node:os';
55
import path from 'node:path';
66
import { afterEach, beforeEach, test } from 'node:test';
77
import { run } from './run.ts';
8+
import {
9+
beginStderrCapture,
10+
beginStdoutCapture,
11+
} from '../../src/__tests__/test-utils/stream-capture.ts';
812

913
let repo: string;
1014

@@ -25,23 +29,14 @@ function writeLcov(records: string): void {
2529
// Redirect the module's stdout/stderr writers so the entrypoint's rendering
2630
// runs without spamming the test reporter.
2731
function capture(fn: () => number): { code: number; out: string } {
28-
const chunks: string[] = [];
29-
const original = process.stdout.write.bind(process.stdout);
30-
const originalErr = process.stderr.write.bind(process.stderr);
31-
process.stdout.write = ((chunk: string | Uint8Array): boolean => {
32-
chunks.push(String(chunk));
33-
return true;
34-
}) as typeof process.stdout.write;
35-
process.stderr.write = ((chunk: string | Uint8Array): boolean => {
36-
chunks.push(String(chunk));
37-
return true;
38-
}) as typeof process.stderr.write;
32+
const stdoutCapture = beginStdoutCapture();
33+
const stderrCapture = beginStderrCapture();
3934
try {
4035
const code = fn();
41-
return { code, out: chunks.join('') };
36+
return { code, out: stdoutCapture.read() + stderrCapture.read() };
4237
} finally {
43-
process.stdout.write = original;
44-
process.stderr.write = originalErr;
38+
stdoutCapture.restore();
39+
stderrCapture.restore();
4540
}
4641
}
4742

‎scripts/coverage-crap/run.test.ts‎

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import os from 'node:os';
55
import path from 'node:path';
66
import { afterEach, beforeEach, test } from 'node:test';
77
import { run } from './run.ts';
8+
import {
9+
beginStderrCapture,
10+
beginStdoutCapture,
11+
} from '../../src/__tests__/test-utils/stream-capture.ts';
812

913
// The verdict is what CI enforces on a PR, so these run the real entrypoint against a real git
1014
// repo, a real fallow and hand-built coverage reports — a stub of fallow could not prove that
@@ -98,43 +102,15 @@ function commit(message: string): void {
98102

99103
// The entrypoint writes reports and reads the job summary, so each case needs the stream capture as
100104
// well as the exit code: `code: 0` with a "no trustworthy verdict" body is the failure mode here.
101-
//
102-
// Restoring means putting back the descriptor, not a function of the same name. `process.stdout`
103-
// inherits `write` from its prototype, so a saved-and-reassigned property would leave an own
104-
// property shadowing the prototype's, and `bind` would have handed back a bound copy no later
105-
// `delete` could undo.
106-
type CapturedStream = Readonly<{
107-
target: NodeJS.WriteStream;
108-
key: 'write';
109-
descriptor: PropertyDescriptor | undefined;
110-
}>;
111-
112105
function capture(fn: () => number): { code: number; out: string } {
113-
const chunks: string[] = [];
114-
const restore: CapturedStream[] = [];
115-
const collect = (chunk: string | Uint8Array): boolean => {
116-
chunks.push(String(chunk));
117-
return true;
118-
};
119-
for (const target of [process.stdout, process.stderr]) {
120-
restore.push({
121-
target,
122-
key: 'write',
123-
descriptor: Object.getOwnPropertyDescriptor(target, 'write'),
124-
});
125-
Object.defineProperty(target, 'write', {
126-
value: collect,
127-
writable: true,
128-
configurable: true,
129-
});
130-
}
106+
const stdoutCapture = beginStdoutCapture();
107+
const stderrCapture = beginStderrCapture();
131108
try {
132-
return { code: fn(), out: chunks.join('') };
109+
const code = fn();
110+
return { code, out: stdoutCapture.read() + stderrCapture.read() };
133111
} finally {
134-
for (const { target, key, descriptor } of restore) {
135-
if (descriptor) Object.defineProperty(target, key, descriptor);
136-
else delete (target as unknown as Record<string, unknown>)[key];
137-
}
112+
stdoutCapture.restore();
113+
stderrCapture.restore();
138114
}
139115
}
140116

‎scripts/fuzz/worker.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
// caught at all: only a separate process keeps that fault off the caller, whose thread may be
88
// the unit lane's test runner (#2053).
99

10+
import { beginStderrCapture } from '../../src/__tests__/test-utils/stream-capture.ts';
1011
import { checkCase, type FuzzFailure } from './invariant.ts';
1112
import { getFuzzTarget } from './registry.ts';
1213

@@ -23,7 +24,7 @@ const target = getFuzzTarget(process.argv[2] ?? '');
2324
// those lines and bury the failure report. Only JavaScript writes go through here: a fatal
2425
// fault writes to the descriptor itself, which is why the runner keeps this process's stderr
2526
// piped rather than discarded.
26-
process.stderr.write = (() => true) as typeof process.stderr.write;
27+
beginStderrCapture({ discard: true });
2728

2829
process.on('message', (request: FuzzWorkerRequest) => {
2930
const failure = checkCase(target, request.input);

‎src/__tests__/cli-capture.ts‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
} from '../daemon-client/daemon-client.ts';
88
import { installIsolatedCliTestEnv } from './cli-test-env.ts';
99
import { mkdtempForTestSync } from '@agent-device/test-kit/tmp-dir';
10+
import { beginStderrCapture, beginStdoutCapture } from './test-utils/stream-capture.ts';
1011

1112
class ExitSignal extends Error {
1213
public readonly code: number;
@@ -55,16 +56,12 @@ export async function runCliCapture(
5556
typeof responderOrOptions === 'function'
5657
? { ...extraOptions, sendToDaemon: responderOrOptions }
5758
: { ...extraOptions, ...(responderOrOptions ?? {}) };
58-
let stdout = '';
59-
let stderr = '';
6059
let code: number | null = null;
6160
const calls: CapturedDaemonRequest[] = [];
6261
const transportOptions: DaemonTransportOptions[] = [];
6362
const stateDir = options.stateDirPrefix ? mkdtempForTestSync(options.stateDirPrefix) : undefined;
6463

6564
const originalExit = process.exit;
66-
const originalStdoutWrite = process.stdout.write.bind(process.stdout);
67-
const originalStderrWrite = process.stderr.write.bind(process.stderr);
6865
const originalCwd = process.cwd();
6966
const restoreEnv = installIsolatedCliTestEnv({
7067
...(options.env ?? {}),
@@ -78,20 +75,12 @@ export async function runCliCapture(
7875
(process as any).exit = ((nextCode?: number) => {
7976
throw new ExitSignal(nextCode ?? 0);
8077
}) as typeof process.exit;
81-
(process.stdout as any).write = ((chunk: unknown, ...args: unknown[]) => {
82-
if (options.passthroughBufferWrites && Buffer.isBuffer(chunk)) {
83-
return originalStdoutWrite(chunk, ...(args as [any]));
84-
}
85-
stdout += String(chunk);
86-
return true;
87-
}) as typeof process.stdout.write;
88-
(process.stderr as any).write = ((chunk: unknown, ...args: unknown[]) => {
89-
if (options.passthroughBufferWrites && Buffer.isBuffer(chunk)) {
90-
return originalStderrWrite(chunk, ...(args as [any]));
91-
}
92-
stderr += String(chunk);
93-
return true;
94-
}) as typeof process.stderr.write;
78+
const stdoutCapture = beginStdoutCapture({
79+
passthroughBufferWrites: options.passthroughBufferWrites,
80+
});
81+
const stderrCapture = beginStderrCapture({
82+
passthroughBufferWrites: options.passthroughBufferWrites,
83+
});
9584

9685
const sendToDaemon = async (
9786
req: CapturedDaemonRequest,
@@ -114,10 +103,16 @@ export async function runCliCapture(
114103
restoreEnv();
115104
if (stateDir) fs.rmSync(stateDir, { recursive: true, force: true });
116105
process.exit = originalExit;
117-
process.stdout.write = originalStdoutWrite;
118-
process.stderr.write = originalStderrWrite;
106+
stdoutCapture.restore();
107+
stderrCapture.restore();
119108
process.chdir(originalCwd);
120109
}
121110

122-
return { code, stdout, stderr, calls, transportOptions };
111+
return {
112+
code,
113+
stdout: stdoutCapture.read(),
114+
stderr: stderrCapture.read(),
115+
calls,
116+
transportOptions,
117+
};
123118
}

0 commit comments

Comments
 (0)