Skip to content

Commit a745e45

Browse files
committed
fix(test): reject reporter exit codes that can wrap to success
1 parent 38cfa87 commit a745e45

8 files changed

Lines changed: 98 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
## Unreleased
44

5+
- Fixed: Custom test reporters reject invalid exit codes, including values such as `256` that
6+
could wrap to success and hide a failing suite. `getExitCode` accepts integers from `0` to `255`
7+
or `undefined`; JSON output reports an invalid code as one `INVALID_ARGS` error.
58
- Fixed: Android `record start` no longer refuses to begin after a reused emulator reassigned the
69
previous recorder's pid. A completed recording's native marker is retired only once its recorder is
710
proven gone, but only an absent pid counted as proof — a pid that now names an unrelated process,
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import assert from 'node:assert/strict';
2+
import fs from 'node:fs/promises';
3+
import path from 'node:path';
4+
import { test } from 'vitest';
5+
import { runCliCapture } from '../../../__tests__/cli-capture.ts';
6+
import { mkdtempForTest } from '../../../__tests__/test-utils/tmp-dir.ts';
7+
8+
test.each([false, true])('CLI refuses a wrapping reporter exit code (json=%s)', async (json) => {
9+
const root = await mkdtempForTest('agent-device-reporter-exit-');
10+
const flow = path.join(root, 'flow.ad');
11+
const reporter = path.join(root, 'reporter.mjs');
12+
await fs.writeFile(flow, 'open Demo\n');
13+
await fs.writeFile(reporter, "export default { name: 'wrapping', getExitCode: () => 256 };\n");
14+
const failed = {
15+
file: flow,
16+
session: 'test:reporter',
17+
status: 'failed',
18+
durationMs: 1,
19+
attempts: 1,
20+
error: { message: 'fixture assertion failed' },
21+
};
22+
const result = await runCliCapture(
23+
['test', flow, '--reporter', reporter, ...(json ? ['--json'] : [])],
24+
async () => ({
25+
ok: true,
26+
data: {
27+
total: 1,
28+
executed: 1,
29+
passed: 0,
30+
failed: 1,
31+
skipped: 0,
32+
notRun: 0,
33+
durationMs: 1,
34+
failures: [failed],
35+
tests: [failed],
36+
},
37+
}),
38+
);
39+
40+
assert.equal(result.calls.length, 1);
41+
assert.equal(result.calls[0]?.command, 'test');
42+
assert.equal(result.code, 1);
43+
if (json) {
44+
const output = JSON.parse(result.stdout);
45+
assert.equal(output.success, false);
46+
assert.equal(output.error.code, 'INVALID_ARGS');
47+
assert.match(output.error.message, /wrapping.*getExitCode.*0 to 255/);
48+
} else {
49+
assert.equal(result.stdout, '');
50+
assert.match(result.stderr, /INVALID_ARGS/);
51+
assert.match(result.stderr, /wrapping.*getExitCode.*0 to 255/);
52+
}
53+
});

src/cli/replay-test/reporters/__tests__/registry.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import assert from 'node:assert/strict';
22
import { test } from 'vitest';
3+
import { AppError } from '@agent-device/kernel/errors';
34
import type { ReplaySuiteResult } from '@agent-device/contracts/replay';
45
import {
56
getReplayTestReporterExitCode,
@@ -252,3 +253,30 @@ test('reporter exit codes can raise but never lower the suite exit code', () =>
252253
1,
253254
);
254255
});
256+
257+
test.each([0, 1, 3, 255, undefined])('preserves valid reporter exit code %s', (code) => {
258+
const reporters: ReplayTestReporter[] = [{ name: 'valid', getExitCode: () => code }];
259+
assert.equal(getReplayTestReporterExitCode(reporters, suite()), code ?? 0);
260+
assert.equal(getReplayTestReporterExitCode(reporters, suite(1)), Math.max(1, code ?? 0));
261+
});
262+
263+
test.each([-1, 0.5, 256, 512, Number.NaN, Infinity, -Infinity, '3', null])(
264+
'rejects invalid reporter exit code %s instead of coercing or wrapping it',
265+
(code) => {
266+
const reporters: ReplayTestReporter[] = [
267+
{ name: 'valid', getExitCode: () => 3 },
268+
{ name: 'invalid', getExitCode: () => code as number },
269+
];
270+
for (const value of [suite(), suite(1)]) {
271+
assert.throws(
272+
() => getReplayTestReporterExitCode(reporters, value),
273+
(error: unknown) =>
274+
error instanceof AppError &&
275+
error.code === 'INVALID_ARGS' &&
276+
error.message.includes('invalid') &&
277+
error.message.includes('getExitCode') &&
278+
error.message.includes('0 to 255'),
279+
);
280+
}
281+
},
282+
);

src/cli/replay-test/reporters/registry.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ReplaySuiteResult } from '@agent-device/contracts/replay';
22
import type { RequestProgressEvent } from '@agent-device/contracts/progress';
3+
import { AppError } from '@agent-device/kernel/errors';
34
import { createCustomReplayTestReporter } from './custom.ts';
45
import { createDefaultReplayTestReporter } from './default.ts';
56
import { getReplayTestExitCode } from './format.ts';
@@ -99,7 +100,14 @@ export function getReplayTestReporterExitCode(
99100
let exitCode = getReplayTestExitCode(suite);
100101
for (const reporter of reporters) {
101102
const reporterExitCode = reporter.getExitCode?.(suite);
102-
if (reporterExitCode !== undefined) exitCode = Math.max(exitCode, reporterExitCode);
103+
if (reporterExitCode === undefined) continue;
104+
if (!Number.isInteger(reporterExitCode) || reporterExitCode < 0 || reporterExitCode > 255) {
105+
throw new AppError(
106+
'INVALID_ARGS',
107+
`Test reporter ${reporter.name} getExitCode must return an integer from 0 to 255 or undefined.`,
108+
);
109+
}
110+
exitCode = Math.max(exitCode, reporterExitCode);
103111
}
104112
return exitCode;
105113
}

src/cli/replay-test/reporters/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ export type ReplayTestReporter = {
7474
onTestStep?(test: ReplayTestStep, context: ReplayTestReporterContext): void;
7575
onTestResult?(test: ReplayTestResult, context: ReplayTestReporterContext): void;
7676
onSuiteEnd?(suite: ReplaySuiteResult, context: ReplayTestReporterContext): void | Promise<void>;
77+
/** Return an integer from 0 to 255, or undefined; a reporter can only raise the suite exit code. */
7778
getExitCode?(suite: ReplaySuiteResult): number | undefined;
7879
};
7980

src/cli/replay-test/reporting.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,12 @@ export async function renderReplayTestResponse(options: {
3838
options.reporterRuntime ??
3939
(await createReplayTestReporterRuntime({ debug, verbose, reporter, reportJunit, json }));
4040
await runReplayTestReporters(runtime.reporters, suite, runtime.context);
41+
const exitCode = getReplayTestReporterExitCode(runtime.reporters, suite);
4142
if (json) {
4243
const { printJson } = await import('../../commands/output/json.ts');
4344
printJson({ success: true, data: suite });
4445
}
45-
return getReplayTestReporterExitCode(runtime.reporters, suite);
46+
return exitCode;
4647
}
4748

4849
export async function createReplayTestReporterRuntime(options: {

src/commands/replay/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ export const testCommandFacet = defineCommandFacet({
246246
text: {
247247
summary: 'Run replay test suites',
248248
cliDetail:
249-
'Relative globs are expanded on the caller from its working directory, whose name is treated literally. Quote glob inputs to defer expansion to test. JUnit reports (--reporter junit:<path>) replace characters forbidden by XML 1.0 with U+FFFD and preserve legal Unicode and whitespace. JSON and other reporters retain the original suite values.',
249+
"Relative globs are expanded on the caller from its working directory, whose name is treated literally. Quote glob inputs to defer expansion to test. JUnit reports (--reporter junit:<path>) replace characters forbidden by XML 1.0 with U+FFFD and preserve legal Unicode and whitespace. JSON and other reporters retain the original suite values. Custom reporter getExitCode hooks must return an integer from 0 to 255 or undefined; the highest valid code wins and cannot lower a failing suite's exit code.",
250250
},
251251
metadata: testCommandMetadata,
252252
run: (client, input) => client.replay.test(withCommandRuntimeHints(input)),

website/docs/docs/replay-e2e.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ export default createReporter;
210210
211211
The CLI loads reporter modules with Node dynamic `import()`. Use `.mjs` or `.js` files at runtime; for TypeScript, compile the reporter to JavaScript before passing it to `--reporter`. Loading `.ts` files directly depends on Node's type-stripping behavior and is not part of the supported reporter contract.
212212
213-
Live reporter hooks are semantic: `onSuiteStart`, `onTestStart`, `onTestStep`, and `onTestResult` run while the daemon request is active; generic command progress frames are not exposed to test reporters. These live hooks are synchronous — they run from the progress stream as events arrive and are not awaited, so keep their work synchronous and defer anything async to `onSuiteEnd`, which the CLI awaits before exiting. `onSuiteEnd` receives the final suite result. `getExitCode` can only raise the suite exit code, never lower it: the highest reporter-provided code wins and failed tests still exit with `1` when no reporter raises it further, so a reporter cannot mask a failing suite.
213+
Live reporter hooks are semantic: `onSuiteStart`, `onTestStart`, `onTestStep`, and `onTestResult` run while the daemon request is active; generic command progress frames are not exposed to test reporters. These live hooks are synchronous — they run from the progress stream as events arrive and are not awaited, so keep their work synchronous and defer anything async to `onSuiteEnd`, which the CLI awaits before exiting. `onSuiteEnd` receives the final suite result. `getExitCode` can only raise the suite exit code, never lower it: the highest reporter-provided code wins and failed tests still exit with `1` when no reporter raises it further, so a reporter cannot mask a failing suite. Return an integer from `0` to `255`, or `undefined` to leave the exit code unchanged. Other values fail with `INVALID_ARGS`; in particular, codes such as `256` are rejected before they can wrap to a successful process exit.
214214
215215
## Parametrise `.ad` scripts
216216

0 commit comments

Comments
 (0)