Skip to content

Commit cefb002

Browse files
thymikeethymikee
committed
test(remote): cover artifact download errors, upload progress output, and Metro stop escalation
Adds the three coverage-based CRAP tests from #3421 (each function now under 30 on the owned-file coverage map) plus the shared src/__tests__/test-utils/stream-capture.ts fake they need, with its regression tests for restore-by-descriptor (no own write/isTTY shadow) and the Buffer-passthrough receiver fix. Covers #3421
1 parent 7da2934 commit cefb002

5 files changed

Lines changed: 613 additions & 23 deletions

File tree

‎src/__tests__/client-metro-startup-cleanup.test.ts‎

Lines changed: 162 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ afterEach(() => {
2626
vi.unstubAllGlobals();
2727
});
2828

29-
test('prepareMetroRuntime stops a spawned Metro process when startup readiness times out', async () => {
30-
const tempRoot = mkdtempForTestSync('agent-device-metro-startup-cleanup-');
29+
function createMetroProject(label: string): string {
30+
const tempRoot = mkdtempForTestSync(`agent-device-metro-startup-cleanup-${label}-`);
3131
const projectRoot = path.join(tempRoot, 'project');
3232
fs.mkdirSync(path.join(projectRoot, 'node_modules'), { recursive: true });
3333
fs.writeFileSync(
@@ -40,39 +40,178 @@ test('prepareMetroRuntime stops a spawned Metro process when startup readiness t
4040
},
4141
}),
4242
);
43+
return projectRoot;
44+
}
4345

44-
vi.mocked(runCmdDetached).mockReturnValue(987);
45-
vi.mocked(waitForProcessExit).mockResolvedValue(true);
46-
const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true);
46+
function metroPrepareInput(projectRoot: string) {
47+
return {
48+
projectRoot,
49+
publicBaseUrl: 'https://public.example.test',
50+
metroPort: 8081,
51+
reuseExisting: true,
52+
installDependenciesIfNeeded: false,
53+
probeTimeoutMs: 10,
54+
startupTimeoutMs: 30_000,
55+
env: { ...process.env, AGENT_DEVICE_DAEMON_AUTH_TOKEN: 'daemon-token' },
56+
} as const;
57+
}
58+
59+
/** Starts a prepare whose Metro never becomes ready, letting readiness time out under fake timers. */
60+
async function runMetroPrepareThroughTimeout(projectRoot: string): Promise<void> {
4761
const fetchMock = vi.fn().mockResolvedValue({
4862
ok: true,
4963
status: 200,
5064
text: async () => 'packager-status:not-running',
5165
});
5266
vi.stubGlobal('fetch', fetchMock);
5367
vi.useFakeTimers();
68+
const preparePromise = prepareMetroRuntime(metroPrepareInput(projectRoot));
69+
const expectedFailure = assert.rejects(
70+
preparePromise,
71+
/React Native dev server did not become ready at http:\/\/127\.0\.0\.1:8081\/status within 30000ms/,
72+
);
73+
await vi.advanceTimersByTimeAsync(30_000);
74+
await expectedFailure;
75+
}
76+
77+
type KillSignal = 'SIGTERM' | 'SIGKILL';
78+
79+
function spyOnKill(behavior: (signal: KillSignal) => boolean): {
80+
signals: Array<[number, string]>;
81+
} {
82+
const signals: Array<[number, string]> = [];
83+
vi.spyOn(process, 'kill').mockImplementation(((pid: number, signal?: string | number) => {
84+
const name = (signal ?? 'SIGTERM') as KillSignal;
85+
signals.push([pid, name]);
86+
return behavior(name) as true;
87+
}) as typeof process.kill);
88+
return { signals };
89+
}
90+
91+
function killError(code: string): NodeJS.ErrnoException {
92+
return Object.assign(new Error(`kill failed: ${code}`), { code });
93+
}
94+
95+
test('prepareMetroRuntime stops a spawned Metro process when startup readiness times out', async () => {
96+
const projectRoot = createMetroProject('timeout');
97+
vi.mocked(runCmdDetached).mockReturnValue(987);
98+
vi.mocked(waitForProcessExit).mockResolvedValue(true);
99+
const kill = spyOnKill(() => true);
54100

55101
try {
56-
const preparePromise = prepareMetroRuntime({
57-
projectRoot,
58-
publicBaseUrl: 'https://public.example.test',
59-
metroPort: 8081,
60-
reuseExisting: true,
61-
installDependenciesIfNeeded: false,
62-
probeTimeoutMs: 10,
63-
startupTimeoutMs: 30_000,
64-
env: { ...process.env, AGENT_DEVICE_DAEMON_AUTH_TOKEN: 'daemon-token' },
102+
await runMetroPrepareThroughTimeout(projectRoot);
103+
assert.equal(vi.mocked(runCmdDetached).mock.calls.length, 1);
104+
assert.deepEqual(kill.signals, [[987, 'SIGTERM']]);
105+
assert.deepEqual(vi.mocked(waitForProcessExit).mock.calls, [[987, 1_000]]);
106+
} finally {
107+
fs.rmSync(path.dirname(projectRoot), { recursive: true, force: true });
108+
}
109+
});
110+
111+
test('stopSpawnedMetroProcess gives up cleanup (best effort) when SIGTERM throws ESRCH or EPERM', async () => {
112+
for (const code of ['ESRCH', 'EPERM']) {
113+
vi.clearAllMocks();
114+
const projectRoot = createMetroProject(`term-${code}`);
115+
vi.mocked(runCmdDetached).mockReturnValue(987);
116+
const kill = spyOnKill(() => {
117+
throw killError(code);
65118
});
66119

67-
const expectedFailure = assert.rejects(
68-
preparePromise,
69-
/React Native dev server did not become ready at http:\/\/127\.0\.0\.1:8081\/status within 30000ms/,
70-
);
71-
await vi.advanceTimersByTimeAsync(30_000);
72-
await expectedFailure;
73-
assert.equal(vi.mocked(runCmdDetached).mock.calls.length, 1);
74-
assert.deepEqual(killSpy.mock.calls[0], [987, 'SIGTERM']);
120+
try {
121+
await runMetroPrepareThroughTimeout(projectRoot);
122+
assert.deepEqual(kill.signals, [[987, 'SIGTERM']], `${code} on SIGTERM must stop cleanup`);
123+
assert.equal(vi.mocked(waitForProcessExit).mock.calls.length, 0, code);
124+
} finally {
125+
fs.rmSync(path.dirname(projectRoot), { recursive: true, force: true });
126+
}
127+
}
128+
});
129+
130+
test('stopSpawnedMetroProcess escalates to SIGKILL when SIGTERM does not exit the process', async () => {
131+
const projectRoot = createMetroProject('escalate');
132+
vi.mocked(runCmdDetached).mockReturnValue(987);
133+
vi.mocked(waitForProcessExit).mockResolvedValue(false);
134+
const kill = spyOnKill(() => true);
135+
136+
try {
137+
await runMetroPrepareThroughTimeout(projectRoot);
138+
assert.deepEqual(kill.signals, [
139+
[987, 'SIGTERM'],
140+
[987, 'SIGKILL'],
141+
]);
142+
assert.deepEqual(vi.mocked(waitForProcessExit).mock.calls, [
143+
[987, 1_000],
144+
[987, 1_000],
145+
]);
146+
} finally {
147+
fs.rmSync(path.dirname(projectRoot), { recursive: true, force: true });
148+
}
149+
});
150+
151+
test('stopSpawnedMetroProcess gives up cleanup (best effort) when SIGKILL throws ESRCH or EPERM', async () => {
152+
for (const code of ['ESRCH', 'EPERM']) {
153+
vi.clearAllMocks();
154+
const projectRoot = createMetroProject(`kill-${code}`);
155+
vi.mocked(runCmdDetached).mockReturnValue(987);
156+
vi.mocked(waitForProcessExit).mockResolvedValue(false);
157+
const kill = spyOnKill((signal) => {
158+
if (signal === 'SIGKILL') throw killError(code);
159+
return true;
160+
});
161+
162+
try {
163+
await runMetroPrepareThroughTimeout(projectRoot);
164+
assert.deepEqual(kill.signals, [
165+
[987, 'SIGTERM'],
166+
[987, 'SIGKILL'],
167+
]);
168+
assert.deepEqual(
169+
vi.mocked(waitForProcessExit).mock.calls,
170+
[[987, 1_000]],
171+
`${code} on SIGKILL means the process is already gone; no second exit wait`,
172+
);
173+
} finally {
174+
fs.rmSync(path.dirname(projectRoot), { recursive: true, force: true });
175+
}
176+
}
177+
});
178+
179+
// The stop call site is `stopSpawnedMetroProcess(pid).catch(() => {})`, so an unexpected kill
180+
// error is swallowed and the only observable outcome is the readiness timeout, exactly as for
181+
// the ESRCH/EPERM give-up cases. These pin the escalation attempt, not a rethrow.
182+
test('prepareMetroRuntime reports the readiness timeout when the escalated SIGKILL is refused with EACCES', async () => {
183+
const projectRoot = createMetroProject('kill-eacces');
184+
vi.mocked(runCmdDetached).mockReturnValue(987);
185+
vi.mocked(waitForProcessExit).mockResolvedValue(false);
186+
const kill = spyOnKill((signal) => {
187+
if (signal === 'SIGKILL') throw killError('EACCES');
188+
return true;
189+
});
190+
191+
try {
192+
await runMetroPrepareThroughTimeout(projectRoot);
193+
assert.deepEqual(kill.signals, [
194+
[987, 'SIGTERM'],
195+
[987, 'SIGKILL'],
196+
]);
197+
assert.deepEqual(vi.mocked(waitForProcessExit).mock.calls, [[987, 1_000]]);
198+
} finally {
199+
fs.rmSync(path.dirname(projectRoot), { recursive: true, force: true });
200+
}
201+
});
202+
203+
test('prepareMetroRuntime reports the readiness timeout when SIGTERM is refused with EACCES', async () => {
204+
const projectRoot = createMetroProject('term-eacces');
205+
vi.mocked(runCmdDetached).mockReturnValue(987);
206+
const kill = spyOnKill(() => {
207+
throw killError('EACCES');
208+
});
209+
210+
try {
211+
await runMetroPrepareThroughTimeout(projectRoot);
212+
assert.deepEqual(kill.signals, [[987, 'SIGTERM']]);
213+
assert.equal(vi.mocked(waitForProcessExit).mock.calls.length, 0);
75214
} finally {
76-
fs.rmSync(tempRoot, { recursive: true, force: true });
215+
fs.rmSync(path.dirname(projectRoot), { recursive: true, force: true });
77216
}
78217
});

‎src/__tests__/test-utils/stream-capture.test.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { test } from 'vitest';
2+
import assert from 'node:assert/strict';
3+
import {
4+
beginStderrCapture,
5+
withCapturedStderr,
6+
withInteractiveStderr,
7+
withStreamTty,
8+
} from './stream-capture.ts';
9+
10+
function hasOwnWrite(stream: NodeJS.WriteStream): boolean {
11+
return Object.prototype.hasOwnProperty.call(stream, 'write');
12+
}
13+
14+
test('withCapturedStderr restores an inherited write without leaving an own property', () => {
15+
const writeBefore = Object.getOwnPropertyDescriptor(process.stderr, 'write');
16+
const captured = withCapturedStderr(() => {
17+
process.stderr.write('into the capture\n');
18+
});
19+
assert.equal(captured, 'into the capture\n');
20+
assert.deepEqual(Object.getOwnPropertyDescriptor(process.stderr, 'write'), writeBefore);
21+
});
22+
23+
test('withCapturedStderr puts back a pre-existing own-property write', () => {
24+
const stream = process.stderr as unknown as Record<string, unknown>;
25+
let sentinelHits = 0;
26+
stream.write = () => {
27+
sentinelHits += 1;
28+
return true;
29+
};
30+
try {
31+
withCapturedStderr(() => {
32+
process.stderr.write('captured\n');
33+
});
34+
assert.equal(hasOwnWrite(process.stderr), true);
35+
process.stderr.write('sentinel\n');
36+
assert.equal(sentinelHits, 1, 'the pre-capture own write must be the live one again');
37+
} finally {
38+
// Success: drop the restored sentinel so the prototype write is live again.
39+
// Failure mid-assert: drop whatever capture stub is still installed.
40+
delete stream.write;
41+
}
42+
});
43+
44+
test('passthroughBufferWrites forwards Buffers to the real stream with the stream as receiver', () => {
45+
const capture = beginStderrCapture({ passthroughBufferWrites: true });
46+
try {
47+
// An empty Buffer reaches the real Writable.write without emitting output; called
48+
// unbound it throws while reading `this._writableState`.
49+
assert.equal(process.stderr.write(Buffer.alloc(0)), true);
50+
} finally {
51+
capture.restore();
52+
}
53+
});
54+
55+
test('withInteractiveStderr hides CI, marks the stream a TTY, and restores all three', () => {
56+
const originalCi = process.env.CI;
57+
const originalIsTTY = Object.getOwnPropertyDescriptor(process.stderr, 'isTTY');
58+
process.env.CI = '1';
59+
try {
60+
const captured = withInteractiveStderr(() => {
61+
assert.equal(process.env.CI, undefined);
62+
assert.equal(process.stderr.isTTY, true);
63+
process.stderr.write('interactive line\n');
64+
});
65+
assert.equal(captured, 'interactive line\n');
66+
assert.equal(process.env.CI, '1');
67+
assert.deepEqual(Object.getOwnPropertyDescriptor(process.stderr, 'isTTY'), originalIsTTY);
68+
assert.deepEqual(Object.getOwnPropertyDescriptor(process.stderr, 'write'), undefined);
69+
} finally {
70+
if (originalCi === undefined) delete process.env.CI;
71+
else process.env.CI = originalCi;
72+
}
73+
});
74+
75+
test('withStreamTty restores the original isTTY descriptor state', () => {
76+
const isTTYBefore = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY');
77+
const toggled = !isTTYBefore?.value;
78+
const sawToggled = withStreamTty(process.stdout, toggled, () => process.stdout.isTTY === toggled);
79+
assert.equal(sawToggled, true);
80+
assert.deepEqual(Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'), isTTYBefore);
81+
});

0 commit comments

Comments
 (0)