diff --git a/CHANGELOG.md b/CHANGELOG.md index 72c50b94b6..d06f28d7eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Fixed: JUnit reports remain readable when replay results contain characters forbidden by XML 1.0, + replacing them with U+FFFD while preserving legal Unicode and whitespace. Original suite values + remain available in JSON and other reporters. - Fixed: `replay export` preserves deep links without `//`, including `tel:` and `mailto:`, as Maestro `openLink` commands in both standalone and app-plus-link `open` actions. - Changed: a command whose synopsis is generated names each option with the label its declaration diff --git a/src/cli/replay-test/reporters/__tests__/junit.test.ts b/src/cli/replay-test/reporters/__tests__/junit.test.ts index 94c654628d..d76e4c1d3c 100644 --- a/src/cli/replay-test/reporters/__tests__/junit.test.ts +++ b/src/cli/replay-test/reporters/__tests__/junit.test.ts @@ -5,6 +5,7 @@ import { test } from 'vitest'; import type { ReplaySuiteResult } from '@agent-device/contracts/replay'; import { parseXmlDocumentSync, type XmlNode } from '@agent-device/xml'; import { createJunitReplayTestReporter } from '../junit.ts'; +import { renderReplayTestResponse } from '../../reporting.ts'; import type { ReplayTestReporterContext } from '../types.ts'; import { mkdtempForTestSync } from '../../../../__tests__/test-utils/tmp-dir.ts'; @@ -107,3 +108,86 @@ test('buildReplayJunitXml escapes tricky skip message', () => { assert.ok(skipped); assert.equal(skipped.attributes.message, TRICKY_TITLE); }); + +async function renderCharacterSuite(value: string): Promise { + const dir = mkdtempForTestSync('agent-device-junit-characters-'); + const reportPath = path.join(dir, 'report.xml'); + const failed = { + file: `/flows/${value}/failed.ad`, + title: value, + session: value, + artifactsDir: value, + status: 'failed' as const, + durationMs: 12, + attempts: 1, + error: { code: 'COMMAND_FAILED' as const, message: value, hint: value }, + }; + const suite: ReplaySuiteResult = { + total: 2, + executed: 1, + passed: 0, + failed: 1, + skipped: 1, + notRun: 0, + durationMs: 12, + failures: [failed], + tests: [ + failed, + { + file: '/flows/skipped.ad', + status: 'skipped', + durationMs: 0, + reason: 'skipped-by-filter', + message: value, + }, + ], + }; + const original = structuredClone(suite); + + const exitCode = await renderReplayTestResponse({ + suite, + reporter: [`junit:${reportPath}`], + }); + + assert.equal(exitCode, 1); + assert.deepEqual(suite, original); + const xml = fs.readFileSync(reportPath, 'utf8'); + assert.doesNotMatch(xml, /="[^"]*[\t\n\r][^"]*"/u, 'XML normalizes raw attribute whitespace'); + assert.doesNotMatch(xml, /\r/u, 'XML normalizes raw carriage returns in text'); + const nodes = parseXmlDocumentSync(xml); + const testsuite = findChild(nodes[0]!, 'testsuite'); + assert.ok(testsuite); + return testsuite; +} + +function assertCharacterValues(testsuite: XmlNode, expected: string): void { + const [failed, skipped] = testsuite.children; + assert.ok(failed); + assert.ok(skipped); + assert.equal(failed.attributes.name, expected); + assert.equal(failed.attributes.classname, `/flows/${expected}`); + assert.equal(failed.attributes.file, `/flows/${expected}/failed.ad`); + const failure = findChild(failed, 'failure'); + assert.equal(failure?.attributes.message, expected); + assert.ok(failure?.text?.startsWith(expected)); + assert.ok(failure?.text?.includes(`hint: ${expected}`)); + const systemOut = findChild(failed, 'system-out'); + assert.ok(systemOut?.text?.includes(`session: ${expected}`)); + assert.ok(systemOut?.text?.includes(`artifactsDir: ${expected}`)); + assert.equal(findChild(skipped, 'skipped')?.attributes.message, expected); +} + +test.each([0x00, 0x08, 0x0b, 0x0c, 0x0e, 0x1b, 0x1f, 0xd800, 0xdfff, 0xfffe, 0xffff])( + 'JUnit replaces XML 1.0 forbidden code point %s without changing the suite result', + async (codePoint) => { + const suite = await renderCharacterSuite(`before${String.fromCodePoint(codePoint)}after`); + assertCharacterValues(suite, 'before\uFFFDafter'); + }, +); + +test('JUnit preserves legal XML whitespace, Unicode boundaries and markup characters', async () => { + const value = + 'before\t\n\r<&"\'�￿\u0020\u007F\u0085\uD7FF\uE000\uFFFD\u{10000}\u{1FFFE}\u{10FFFF}after'; + const suite = await renderCharacterSuite(value); + assertCharacterValues(suite, value); +}); diff --git a/src/cli/replay-test/reporters/junit.ts b/src/cli/replay-test/reporters/junit.ts index 33ff0312b8..d705614e52 100644 --- a/src/cli/replay-test/reporters/junit.ts +++ b/src/cli/replay-test/reporters/junit.ts @@ -67,11 +67,11 @@ function buildReplayJunitXml(suite: ReplaySuiteResult): string { } function renderJUnitTestCase(test: ReplaySuiteTestResult): string[] { - const name = escapeXmlTextAndAttribute(replayTestCaseName(test)); - const className = escapeXmlTextAndAttribute( + const name = escapeJunitXml(replayTestCaseName(test)); + const className = escapeJunitXml( `${path.dirname(test.file) === '.' ? test.file : path.dirname(test.file)}${formatReplayTestShardSuffix(test)}`, ); - const file = escapeXmlTextAndAttribute(test.file); + const file = escapeJunitXml(test.file); const time = formatJUnitSeconds(test.durationMs); const lines = [ ` `, @@ -79,21 +79,34 @@ function renderJUnitTestCase(test: ReplaySuiteTestResult): string[] { if (test.status === 'failed') { lines.push( - ` ${escapeXmlTextAndAttribute(buildFailureDetails(test))}`, + ` ${escapeJunitXml(buildFailureDetails(test))}`, ); } else if (test.status === 'skipped') { - lines.push(` `); + lines.push(` `); } const systemOut = buildSystemOut(test); if (systemOut) { - lines.push(` ${escapeXmlTextAndAttribute(systemOut)}`); + lines.push(` ${escapeJunitXml(systemOut)}`); } lines.push(' '); return lines; } +function escapeJunitXml(value: string): string { + // XML 1.0 cannot represent these characters, even as numeric character references. + const representable = value.replaceAll( + /[^\t\n\r\u0020-\uD7FF\uE000-\uFFFD\u{10000}-\u{10FFFF}]/gu, + '\uFFFD', + ); + // Character references preserve whitespace through XML attribute and line-end normalization. + return escapeXmlTextAndAttribute(representable) + .replaceAll('\t', ' ') + .replaceAll('\n', ' ') + .replaceAll('\r', ' '); +} + function buildFailureDetails(test: FailedReplayTestResult): string { const lines = [test.error.message]; appendReplayErrorMetadata(lines, test.error, { includeDetails: false }); diff --git a/src/commands/replay/index.ts b/src/commands/replay/index.ts index da5979d43f..c300466903 100644 --- a/src/commands/replay/index.ts +++ b/src/commands/replay/index.ts @@ -245,6 +245,8 @@ export const testCommandFacet = defineCommandFacet({ name: TEST_COMMAND_NAME, text: { summary: 'Run replay test suites', + cliDetail: + 'JUnit reports (--reporter junit:) 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.', }, metadata: testCommandMetadata, run: (client, input) => client.replay.test(withCommandRuntimeHints(input)), diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index 1720739b9f..62fd2bf9b1 100644 --- a/website/docs/docs/replay-e2e.md +++ b/website/docs/docs/replay-e2e.md @@ -122,6 +122,7 @@ agent-device test ./workflows --reporter default --reporter junit:./tmp/junit.xm - Timeouts are cooperative: the runner marks the attempt failed at the timeout boundary, then gives the underlying replay a short grace period to stop before session cleanup. - The default text reporter streams live progress on stderr while a suite runs, then prints the final summary, failed tests, and passed-on-retry flaky tests. Use `--verbose` to include step traces in completed-test progress output. - `--reporter` is repeatable. Built-ins are `default` for the console summary and `junit:` for JUnit XML. Passing any explicit reporter list replaces the implicit default reporter, so include `--reporter default` when you also want terminal output. `--report-junit ` remains a compatibility alias for `--reporter junit:`. +- JUnit reports preserve legal Unicode and whitespace, and replace characters forbidden by XML 1.0 (such as terminal ESC or NUL) with `U+FFFD` (`�`) so CI parsers can read the report. JSON and other reporters retain the original suite values. - When `--fail-fast` and retries are both set, the current test still consumes its retries before the suite stops. ### Custom test reporters