From 9c9c54e7570401be885d3bfa363f703f208fa245 Mon Sep 17 00:00:00 2001 From: Ash Wu Date: Tue, 28 Apr 2026 21:38:37 +0800 Subject: [PATCH 1/3] [build-tools] Fix ai-*.json is not guarenteed in Maestro 2.5 --- .../__tests__/maestroFlowDiscovery.test.ts | 302 +++++++++++++ .../__tests__/maestroResultParser.test.ts | 410 +++--------------- .../reportMaestroTestResults.test.ts | 8 +- .../steps/functions/maestroFlowDiscovery.ts | 200 +++++++++ .../steps/functions/maestroResultParser.ts | 224 ++-------- .../src/steps/functions/maestroTests.ts | 57 ++- .../functions/reportMaestroTestResults.ts | 33 +- 7 files changed, 656 insertions(+), 578 deletions(-) create mode 100644 packages/build-tools/src/steps/functions/__tests__/maestroFlowDiscovery.test.ts create mode 100644 packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts diff --git a/packages/build-tools/src/steps/functions/__tests__/maestroFlowDiscovery.test.ts b/packages/build-tools/src/steps/functions/__tests__/maestroFlowDiscovery.test.ts new file mode 100644 index 0000000000..cfcc0abac7 --- /dev/null +++ b/packages/build-tools/src/steps/functions/__tests__/maestroFlowDiscovery.test.ts @@ -0,0 +1,302 @@ +import { vol } from 'memfs'; + +import { buildFlowNameToPathMap, readFlowName, walkDir } from '../maestroFlowDiscovery'; +import { createMockLogger } from '../../../__tests__/utils/logger'; + +describe(readFlowName, () => { + beforeEach(() => vol.reset()); + + it('returns top-level `name` when present', async () => { + vol.fromJSON({ + '/p/login.yml': `appId: com.example\nname: Login Flow\n---\n- launchApp\n`, + }); + expect(await readFlowName('/p/login.yml')).toBe('Login Flow'); + }); + + it('falls back to basename without extension when name absent', async () => { + vol.fromJSON({ '/p/login.yml': `appId: com.example\n---\n- launchApp\n` }); + expect(await readFlowName('/p/login.yml')).toBe('login'); + }); + + it('falls back when name is empty string', async () => { + vol.fromJSON({ '/p/login.yml': `name: ""\n` }); + expect(await readFlowName('/p/login.yml')).toBe('login'); + }); + + it('falls back when name is non-string', async () => { + vol.fromJSON({ '/p/login.yml': `name: 42\n` }); + expect(await readFlowName('/p/login.yml')).toBe('login'); + }); + + it('falls back when YAML is malformed', async () => { + vol.fromJSON({ '/p/login.yml': `appId: [unclosed\n` }); + expect(await readFlowName('/p/login.yml')).toBe('login'); + }); + + it('falls back when file is empty', async () => { + vol.fromJSON({ '/p/login.yml': '' }); + expect(await readFlowName('/p/login.yml')).toBe('login'); + }); + + it('falls back when file does not exist', async () => { + expect(await readFlowName('/p/missing.yml')).toBe('missing'); + }); + + it('only parses the first YAML document (commands document errors are ignored)', async () => { + vol.fromJSON({ + '/p/login.yml': `name: Login\n---\n- this is: { malformed yaml\n`, + }); + expect(await readFlowName('/p/login.yml')).toBe('Login'); + }); + + it('handles .yaml extension', async () => { + vol.fromJSON({ '/p/checkout.yaml': `name: Checkout\n` }); + expect(await readFlowName('/p/checkout.yaml')).toBe('Checkout'); + }); + + it('handles a leading `---` document marker', async () => { + vol.fromJSON({ + '/p/login.yml': `---\nappId: com.example\nname: Login\n---\n- launchApp\n`, + }); + expect(await readFlowName('/p/login.yml')).toBe('Login'); + }); +}); + +describe(walkDir, () => { + beforeEach(() => vol.reset()); + + it('collects all .yml/.yaml files recursively, sorted', async () => { + const logger = createMockLogger(); + vol.fromJSON({ + '/p/flows/a.yml': '', + '/p/flows/b.yaml': '', + '/p/flows/sub/c.yml': '', + '/p/flows/sub/d.YAML': '', + '/p/flows/readme.md': '', + }); + const out: string[] = []; + await walkDir('/p/flows', new Set(), out, logger); + expect(out.sort()).toEqual([ + '/p/flows/a.yml', + '/p/flows/b.yaml', + '/p/flows/sub/c.yml', + '/p/flows/sub/d.YAML', + ]); + }); + + it('skips files named config.yaml (case-insensitive)', async () => { + const logger = createMockLogger(); + vol.fromJSON({ + '/p/flows/config.yaml': '', + '/p/flows/CONFIG.yaml': '', + '/p/flows/login.yml': '', + }); + const out: string[] = []; + await walkDir('/p/flows', new Set(), out, logger); + expect(out.sort()).toEqual(['/p/flows/login.yml']); + }); + + it('returns silently when realpath fails (e.g. missing dir)', async () => { + const logger = createMockLogger(); + const out: string[] = []; + await walkDir('/p/missing', new Set(), out, logger); + expect(out).toEqual([]); + }); +}); + +describe(buildFlowNameToPathMap, () => { + beforeEach(() => vol.reset()); + + it('builds a map from a single file input (basename name)', async () => { + const logger = createMockLogger(); + vol.fromJSON({ '/proj/flows/login.yml': 'appId: x\n' }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['flows/login.yml'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual(new Map([['login', 'flows/login.yml']])); + }); + + it('uses top-level name when present', async () => { + const logger = createMockLogger(); + vol.fromJSON({ '/proj/flows/a.yml': 'name: Login Flow\n' }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['flows/a.yml'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual(new Map([['Login Flow', 'flows/a.yml']])); + }); + + it('walks a directory input and adds each flow', async () => { + const logger = createMockLogger(); + vol.fromJSON({ + '/proj/.maestro/login.yml': 'name: Login\n', + '/proj/.maestro/checkout.yml': '', + '/proj/.maestro/sub/profile.yml': '', + }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['.maestro'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual( + new Map([ + ['Login', '.maestro/login.yml'], + ['checkout', '.maestro/checkout.yml'], + ['profile', '.maestro/sub/profile.yml'], + ]) + ); + }); + + it('merges file + directory inputs', async () => { + const logger = createMockLogger(); + vol.fromJSON({ + '/proj/.maestro/a.yml': '', + '/proj/extra/b.yml': '', + }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['.maestro', 'extra/b.yml'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual( + new Map([ + ['a', '.maestro/a.yml'], + ['b', 'extra/b.yml'], + ]) + ); + }); + + it('skips non-existent inputs with warn', async () => { + const logger = createMockLogger(); + vol.fromJSON({ '/proj/flows/a.yml': '' }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['flows/missing.yml', 'flows/a.yml'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual(new Map([['a', 'flows/a.yml']])); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('flows/missing.yml')); + }); + + it('skips workspace config.yaml inside directory inputs', async () => { + const logger = createMockLogger(); + vol.fromJSON({ + '/proj/.maestro/config.yaml': 'flows:\n - "*.yml"\n', + '/proj/.maestro/login.yml': '', + }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['.maestro'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual(new Map([['login', '.maestro/login.yml']])); + }); + + it('does NOT skip config.yaml when passed as an explicit file input', async () => { + const logger = createMockLogger(); + vol.fromJSON({ + '/proj/.maestro/config.yaml': 'name: Workspace Defaults\nflows:\n - "*.yml"\n', + }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['.maestro/config.yaml'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual(new Map([['Workspace Defaults', '.maestro/config.yaml']])); + }); + + it('returns null when two files share top-level name', async () => { + const logger = createMockLogger(); + vol.fromJSON({ + '/proj/a.yml': 'name: Login\n', + '/proj/b.yml': 'name: Login\n', + }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['a.yml', 'b.yml'], + projectRoot: '/proj', + logger, + }); + expect(map).toBeNull(); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('"Login"')); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('a.yml')); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('b.yml')); + }); + + it('returns null when two files share basename across directories', async () => { + const logger = createMockLogger(); + vol.fromJSON({ + '/proj/login/index.yml': '', + '/proj/checkout/index.yml': '', + }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['login', 'checkout'], + projectRoot: '/proj', + logger, + }); + expect(map).toBeNull(); + }); + + it('returns null when name collides with another file basename', async () => { + const logger = createMockLogger(); + vol.fromJSON({ + '/proj/login.yml': 'name: Other\n', + '/proj/Other.yml': '', + }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['login.yml', 'Other.yml'], + projectRoot: '/proj', + logger, + }); + expect(map).toBeNull(); + }); + + it('dedupes the same physical file passed via overlapping inputs', async () => { + const logger = createMockLogger(); + vol.fromJSON({ '/proj/.maestro/login.yml': '' }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['.maestro', '.maestro/login.yml'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual(new Map([['login', '.maestro/login.yml']])); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('uses absolute fallback when input file lives outside projectRoot', async () => { + const logger = createMockLogger(); + vol.fromJSON({ '/elsewhere/a.yml': '' }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['/elsewhere/a.yml'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual(new Map([['a', '/elsewhere/a.yml']])); + }); + + it('absolute directory input under projectRoot yields project-relative children', async () => { + const logger = createMockLogger(); + vol.fromJSON({ '/proj/.maestro/a.yml': '' }); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: ['/proj/.maestro'], + projectRoot: '/proj', + logger, + }); + expect(map).toEqual(new Map([['a', '.maestro/a.yml']])); + }); + + it('returns null and warns on unexpected helper failure', async () => { + const logger = createMockLogger(); + const map = await buildFlowNameToPathMap({ + inputFlowPaths: [Symbol('bad') as unknown as string], + projectRoot: '/proj', + logger, + }); + expect(map).toBeNull(); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('buildFlowNameToPathMap failed unexpectedly') + ); + }); +}); diff --git a/packages/build-tools/src/steps/functions/__tests__/maestroResultParser.test.ts b/packages/build-tools/src/steps/functions/__tests__/maestroResultParser.test.ts index 1bd1b29bfe..96f49b551c 100644 --- a/packages/build-tools/src/steps/functions/__tests__/maestroResultParser.test.ts +++ b/packages/build-tools/src/steps/functions/__tests__/maestroResultParser.test.ts @@ -6,63 +6,13 @@ import { copyLatestAttemptXml, mergeJUnitReports, parseFailedFlowsFromJUnit, - parseFlowMetadata, parseJUnitTestCases, parseMaestroResults, } from '../maestroResultParser'; -describe(parseFlowMetadata, () => { - it('parses valid ai-*.json', async () => { - vol.fromJSON({ - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/Users/expo/workingdir/build/.maestro/home.yml', - }), - }); - - const result = await parseFlowMetadata('/tests/2026-01-28_055409/ai-home.json'); - expect(result).toEqual({ - flow_name: 'home', - flow_file_path: '/Users/expo/workingdir/build/.maestro/home.yml', - }); - }); - - it('returns null when flow_name is missing', async () => { - vol.fromJSON({ - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_file_path: '/Users/expo/workingdir/build/.maestro/home.yml', - }), - }); - - const result = await parseFlowMetadata('/tests/2026-01-28_055409/ai-home.json'); - expect(result).toBeNull(); - }); - - it('returns null when flow_file_path is missing', async () => { - vol.fromJSON({ - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - }), - }); - - const result = await parseFlowMetadata('/tests/2026-01-28_055409/ai-home.json'); - expect(result).toBeNull(); - }); - - it('returns null for invalid JSON', async () => { - vol.fromJSON({ - '/tests/2026-01-28_055409/ai-home.json': 'not json', - }); - - const result = await parseFlowMetadata('/tests/2026-01-28_055409/ai-home.json'); - expect(result).toBeNull(); - }); -}); - describe(parseMaestroResults, () => { - it('parses JUnit results and enriches with ai-*.json metadata', async () => { + it('parses JUnit results and enriches with name→path map', async () => { vol.fromJSON({ - // JUnit XML (primary data) '/junit/report.xml': [ '', '', @@ -74,18 +24,15 @@ describe(parseMaestroResults, () => { ' ', '', ].join('\n'), - // Debug output (for flow_file_path) - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/tests/2026-01-28_055409/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), }); - const results = await parseMaestroResults('/junit', '/tests', '/root/project'); + const results = await parseMaestroResults( + '/junit', + new Map([ + ['home', '.maestro/home.yml'], + ['login', '.maestro/login.yml'], + ]) + ); expect(results).toHaveLength(2); expect(results).toEqual( expect.arrayContaining([ @@ -113,7 +60,7 @@ describe(parseMaestroResults, () => { ); }); - it('calculates retryCount from timestamp directory occurrences', async () => { + it('uses flow name as fallback path when nameToPath is null', async () => { vol.fromJSON({ '/junit/report.xml': [ '', @@ -123,22 +70,14 @@ describe(parseMaestroResults, () => { ' ', '', ].join('\n'), - // Two timestamp dirs = 1 retry - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/tests/2026-01-28_055420/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), }); - const results = await parseMaestroResults('/junit', '/tests', '/root/project'); - expect(results[0].retryCount).toBe(1); + const results = await parseMaestroResults('/junit', null); + expect(results[0].path).toBe('home'); + expect(results[0].retryCount).toBe(0); }); - it('uses flow name as fallback path when ai-*.json not found', async () => { + it('uses flow name as fallback path when name not in map', async () => { vol.fromJSON({ '/junit/report.xml': [ '', @@ -148,24 +87,16 @@ describe(parseMaestroResults, () => { ' ', '', ].join('\n'), - // No ai-*.json files }); - const results = await parseMaestroResults('/junit', '/tests', '/root/project'); + const results = await parseMaestroResults('/junit', new Map()); expect(results[0].path).toBe('home'); - expect(results[0].retryCount).toBe(0); }); it('returns empty array when no JUnit files found', async () => { - vol.fromJSON({ - '/junit/.gitkeep': '', - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - }); + vol.fromJSON({ '/junit/.gitkeep': '' }); - const results = await parseMaestroResults('/junit', '/tests', '/root/project'); + const results = await parseMaestroResults('/junit', new Map([['home', '.maestro/home.yml']])); expect(results).toEqual([]); }); @@ -184,44 +115,13 @@ describe(parseMaestroResults, () => { ' ', '', ].join('\n'), - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), }); - const results = await parseMaestroResults('/junit', '/tests', '/root/project'); + const results = await parseMaestroResults('/junit', new Map([['home', '.maestro/home.yml']])); expect(results[0].tags).toEqual(['e2e', 'smoke']); expect(results[0].properties).toEqual({ env: 'staging' }); }); - it('handles reuse_devices=true (junit_report_directory == tests_directory)', async () => { - vol.fromJSON({ - // Same directory for both JUnit and debug output - '/maestro-tests/android-maestro-junit.xml': [ - '', - '', - ' ', - ' ', - ' ', - '', - ].join('\n'), - '/maestro-tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - }); - - const results = await parseMaestroResults('/maestro-tests', '/maestro-tests', '/root/project'); - expect(results).toEqual([ - expect.objectContaining({ - name: 'home', - path: '.maestro/home.yml', - status: 'passed', - }), - ]); - }); - it('returns per-attempt results when multiple JUnit files exist for the same flow', async () => { vol.fromJSON({ // Attempt 0: login FAILED @@ -244,20 +144,11 @@ describe(parseMaestroResults, () => { ' ', '', ].join('\n'), - // ai-*.json metadata (2 timestamp dirs = 2 attempts) - '/tests/2026-01-28_055409/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), - '/tests/2026-01-28_055420/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), }); - const results = await parseMaestroResults('/junit', '/tests', '/root/project'); + const results = await parseMaestroResults('/junit', new Map([['login', '.maestro/login.yml']])); - // Should return 2 results — one per attempt + // Should return 2 results — one per attempt; retryCount from filename expect(results).toHaveLength(2); expect(results).toEqual([ expect.objectContaining({ @@ -303,25 +194,15 @@ describe(parseMaestroResults, () => { ' ', '', ].join('\n'), - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/tests/2026-01-28_055409/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), - '/tests/2026-01-28_055420/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/tests/2026-01-28_055420/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), }); - const results = await parseMaestroResults('/junit-reports', '/tests', '/root/project'); + const results = await parseMaestroResults( + '/junit-reports', + new Map([ + ['home', '.maestro/home.yml'], + ['login', '.maestro/login.yml'], + ]) + ); // 4 results: 2 flows × 2 attempts expect(results).toHaveLength(4); @@ -335,7 +216,6 @@ describe(parseMaestroResults, () => { it('returns per-attempt results with sharding (multiple testsuites per attempt file)', async () => { vol.fromJSON({ - // Attempt 0: shard 1 has home (FAILED), shard 2 has login (PASSED) '/junit-reports/android-maestro-junit-attempt-0.xml': [ '', '', @@ -349,7 +229,6 @@ describe(parseMaestroResults, () => { ' ', '', ].join('\n'), - // Attempt 1: all passed across shards '/junit-reports/android-maestro-junit-attempt-1.xml': [ '', '', @@ -361,25 +240,15 @@ describe(parseMaestroResults, () => { ' ', '', ].join('\n'), - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/tests/2026-01-28_055409/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), - '/tests/2026-01-28_055420/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/tests/2026-01-28_055420/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), }); - const results = await parseMaestroResults('/junit-reports', '/tests', '/root/project'); + const results = await parseMaestroResults( + '/junit-reports', + new Map([ + ['home', '.maestro/home.yml'], + ['login', '.maestro/login.yml'], + ]) + ); expect(results).toHaveLength(4); expect(results).toEqual([ @@ -390,52 +259,8 @@ describe(parseMaestroResults, () => { ]); }); - it('backward compat: reuse_devices=true with retries but old single JUnit file', async () => { + it('handles reuse_devices=false (separate junit_report_directory, per-flow files)', async () => { vol.fromJSON({ - // Single overwritten JUnit (only has final attempt's results) - '/maestro-tests/android-maestro-junit.xml': [ - '', - '', - ' ', - ' ', - ' ', - ' ', - '', - ].join('\n'), - // 2 timestamp dirs — both flows appear in both (entire suite retried) - '/maestro-tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/maestro-tests/2026-01-28_055409/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), - '/maestro-tests/2026-01-28_055420/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/maestro-tests/2026-01-28_055420/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), - }); - - const results = await parseMaestroResults('/maestro-tests', '/maestro-tests', '/root/project'); - - // Both flows have 2 occurrences → retryCount = 1 for both - expect(results).toHaveLength(2); - expect(results).toEqual( - expect.arrayContaining([ - expect.objectContaining({ name: 'home', status: 'passed', retryCount: 1 }), - expect.objectContaining({ name: 'login', status: 'passed', retryCount: 1 }), - ]) - ); - }); - - it('handles reuse_devices=false (separate junit_report_directory)', async () => { - vol.fromJSON({ - // JUnit in temp dir (per-flow files) '/tmp/maestro-reports-abc123/junit-report-flow-1.xml': [ '', '', @@ -454,21 +279,14 @@ describe(parseMaestroResults, () => { ' ', '', ].join('\n'), - // Debug output in default location - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/tests/2026-01-28_055409/ai-login.json': JSON.stringify({ - flow_name: 'login', - flow_file_path: '/root/project/.maestro/login.yml', - }), }); const results = await parseMaestroResults( '/tmp/maestro-reports-abc123', - '/tests', - '/root/project' + new Map([ + ['home', '.maestro/home.yml'], + ['login', '.maestro/login.yml'], + ]) ); expect(results).toHaveLength(2); expect(results).toEqual( @@ -478,51 +296,6 @@ describe(parseMaestroResults, () => { ]) ); }); - - it('uses raw path when flow_file_path is outside project_root', async () => { - vol.fromJSON({ - '/junit/report.xml': [ - '', - '', - ' ', - ' ', - ' ', - '', - ].join('\n'), - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/somewhere/else/.maestro/home.yml', - }), - }); - - const results = await parseMaestroResults('/junit', '/tests', '/root/project'); - expect(results[0].path).toBe('/somewhere/else/.maestro/home.yml'); - }); - - it('filters out non-timestamp directories when scanning debug output', async () => { - vol.fromJSON({ - '/junit/report.xml': [ - '', - '', - ' ', - ' ', - ' ', - '', - ].join('\n'), - '/tests/2026-01-28_055409/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - '/tests/not-a-timestamp/ai-home.json': JSON.stringify({ - flow_name: 'home', - flow_file_path: '/root/project/.maestro/home.yml', - }), - }); - - const results = await parseMaestroResults('/junit', '/tests', '/root/project'); - // Only 1 occurrence (not-a-timestamp dir should be ignored) - expect(results[0].retryCount).toBe(0); - }); }); describe(parseJUnitTestCases, () => { @@ -811,11 +584,7 @@ describe(parseJUnitTestCases, () => { describe('parseFailedFlowsFromJUnit', () => { it('returns the subset of input flow paths whose testcases failed', async () => { - // memfs setup: 3 input flows, 1 junit file with 1 failure, 1 timestamp dir with 3 ai-*.json vol.fromJSON({ - '/project/flows/login.yaml': '', - '/project/flows/search.yaml': '', - '/project/flows/checkout.yaml': '', '/tmp/junit-reports/android-maestro-junit-attempt-0.xml': ` @@ -824,82 +593,49 @@ describe('parseFailedFlowsFromJUnit', () => { something `, - '/tmp/tests/2026-04-23_120000/ai-Login.json': JSON.stringify({ - flow_name: 'Login', - flow_file_path: '/project/flows/login.yaml', - }), - '/tmp/tests/2026-04-23_120000/ai-Search.json': JSON.stringify({ - flow_name: 'Search', - flow_file_path: '/project/flows/search.yaml', - }), - '/tmp/tests/2026-04-23_120000/ai-Checkout.json': JSON.stringify({ - flow_name: 'Checkout', - flow_file_path: '/project/flows/checkout.yaml', - }), }); const result = await parseFailedFlowsFromJUnit({ junitFile: '/tmp/junit-reports/android-maestro-junit-attempt-0.xml', - testsDirectory: '/tmp/tests', - inputFlowPaths: ['flows/login.yaml', 'flows/search.yaml', 'flows/checkout.yaml'], - projectRoot: '/project', + nameToPath: new Map([ + ['Login', 'flows/login.yaml'], + ['Search', 'flows/search.yaml'], + ['Checkout', 'flows/checkout.yaml'], + ]), }); expect(result).toEqual(['flows/checkout.yaml']); }); - it('accepts flow files discovered under a directory input (documented usage)', async () => { - // Users may set `flow_path: ./maestro/flows` (a directory). Maestro then - // discovers the YAMLs inside; smart retry must still subset to the failing - // child file, not fall back to dumb retry. + it('returns null when a failing testcase has no entry in nameToPath', async () => { vol.fromJSON({ - '/project/flows/login.yaml': '', - '/project/flows/checkout.yaml': '', '/tmp/junit-reports/attempt-0.xml': ` - x + x `, - '/tmp/tests/2026-04-23_120000/ai-Login.json': JSON.stringify({ - flow_name: 'Login', - flow_file_path: '/project/flows/login.yaml', - }), - '/tmp/tests/2026-04-23_120000/ai-Checkout.json': JSON.stringify({ - flow_name: 'Checkout', - flow_file_path: '/project/flows/checkout.yaml', - }), }); const result = await parseFailedFlowsFromJUnit({ junitFile: '/tmp/junit-reports/attempt-0.xml', - testsDirectory: '/tmp/tests', - inputFlowPaths: ['flows'], - projectRoot: '/project', + nameToPath: new Map([['Login', 'flows/login.yaml']]), }); - expect(result).toEqual(['flows/checkout.yaml']); + expect(result).toBeNull(); }); it('returns null when two failing testcases share the same name (collision)', async () => { vol.fromJSON({ - '/project/flows/a.yaml': '', - '/project/flows/b.yaml': '', '/tmp/junit-reports/android-maestro-junit-attempt-0.xml': ` x y `, - '/tmp/tests/2026-04-23_120000/ai-Duplicate.json': JSON.stringify({ - flow_name: 'Duplicate', - flow_file_path: '/project/flows/a.yaml', - }), }); const result = await parseFailedFlowsFromJUnit({ junitFile: '/tmp/junit-reports/android-maestro-junit-attempt-0.xml', - testsDirectory: '/tmp/tests', - inputFlowPaths: ['flows/a.yaml', 'flows/b.yaml'], - projectRoot: '/project', + nameToPath: new Map([['Duplicate', 'flows/a.yaml']]), }); expect(result).toBeNull(); @@ -907,38 +643,25 @@ describe('parseFailedFlowsFromJUnit', () => { it('returns null when a failing testcase shares a name with any other testcase (pass or fail)', async () => { vol.fromJSON({ - '/project/flows/a.yaml': '', - '/project/flows/b.yaml': '', '/tmp/junit-reports/android-maestro-junit-attempt-0.xml': ` x `, - '/tmp/tests/2026-04-23_120000/ai-Shared.json': JSON.stringify({ - flow_name: 'Shared', - flow_file_path: '/project/flows/a.yaml', - }), }); const result = await parseFailedFlowsFromJUnit({ junitFile: '/tmp/junit-reports/android-maestro-junit-attempt-0.xml', - testsDirectory: '/tmp/tests', - inputFlowPaths: ['flows/a.yaml', 'flows/b.yaml'], - projectRoot: '/project', + nameToPath: new Map([['Shared', 'flows/a.yaml']]), }); expect(result).toBeNull(); }); it('returns null when junit file does not exist', async () => { - vol.fromJSON({ - '/project/flows/a.yaml': '', - }); const result = await parseFailedFlowsFromJUnit({ junitFile: '/tmp/missing.xml', - testsDirectory: '/tmp/tests', - inputFlowPaths: ['flows/a.yaml'], - projectRoot: '/project', + nameToPath: new Map([['A', 'flows/a.yaml']]), }); expect(result).toBeNull(); }); @@ -949,9 +672,7 @@ describe('parseFailedFlowsFromJUnit', () => { }); const result = await parseFailedFlowsFromJUnit({ junitFile: '/tmp/junit-reports/bad.xml', - testsDirectory: '/tmp/tests', - inputFlowPaths: ['flows/a.yaml'], - projectRoot: '/project', + nameToPath: new Map([['A', 'flows/a.yaml']]), }); expect(result).toBeNull(); }); @@ -959,50 +680,33 @@ describe('parseFailedFlowsFromJUnit', () => { it('returns null when junit XML is truncated mid-tag (partial parse risk)', async () => { // fast-xml-parser can produce a partial parse from truncated XML — without // strict validation, smart retry would only retry the visible failures and - // silently skip flows that were cut off, masking real failures when the - // subset retry passes. Validation must reject the file → dumb retry. + // silently skip flows that were cut off. vol.fromJSON({ - '/project/flows/a.yaml': '', - '/project/flows/b.yaml': '', '/tmp/junit-reports/attempt-0.xml': ` x { vol.fromJSON({ - '/project/flows/a.yaml': '', '/tmp/junit-reports/attempt-0.xml': ` x `, // missing - '/tmp/tests/2026-04-23_120000/ai-A.json': JSON.stringify({ - flow_name: 'A', - flow_file_path: '/project/flows/a.yaml', - }), }); const result = await parseFailedFlowsFromJUnit({ junitFile: '/tmp/junit-reports/attempt-0.xml', - testsDirectory: '/tmp/tests', - inputFlowPaths: ['flows/a.yaml'], - projectRoot: '/project', + nameToPath: new Map([['A', 'flows/a.yaml']]), }); expect(result).toBeNull(); }); diff --git a/packages/build-tools/src/steps/functions/__tests__/reportMaestroTestResults.test.ts b/packages/build-tools/src/steps/functions/__tests__/reportMaestroTestResults.test.ts index b46d05f199..908fa38c51 100644 --- a/packages/build-tools/src/steps/functions/__tests__/reportMaestroTestResults.test.ts +++ b/packages/build-tools/src/steps/functions/__tests__/reportMaestroTestResults.test.ts @@ -72,7 +72,7 @@ describe(createReportMaestroTestResultsFunction, () => { it('parses JUnit results and calls GraphQL mutation', async () => { vol.fromJSON({ '/junit/report.xml': JUNIT_PASS, - '/tests/2026-01-28_055409/ai-home.json': FLOW_AI, + '/root/project/.maestro/home.yml': '', }); mockMutationFn.mockResolvedValue({ @@ -83,7 +83,11 @@ describe(createReportMaestroTestResultsFunction, () => { }, }); - await createStep().executeAsync(); + await createStep({ + callInputs: { + flow_paths: ['.maestro/home.yml'], + }, + }).executeAsync(); expect(mockGraphqlClient.mutation).toHaveBeenCalledTimes(1); const [, variables] = (mockGraphqlClient.mutation as jest.Mock).mock.calls[0]; diff --git a/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts b/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts new file mode 100644 index 0000000000..c3f101820a --- /dev/null +++ b/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts @@ -0,0 +1,200 @@ +import type { bunyan } from '@expo/logger'; +import fs from 'fs/promises'; +import path from 'path'; +import yaml from 'yaml'; + +const YAML_EXT = /\.ya?ml$/i; +const WORKSPACE_CONFIG_BASENAME = 'config.yaml'; + +export interface BuildFlowNameToPathMapArgs { + inputFlowPaths: string[]; + projectRoot: string; + logger: bunyan; +} + +interface FlowEntry { + name: string; + path: string; + realpath: string; +} + +export async function buildFlowNameToPathMap( + args: BuildFlowNameToPathMapArgs +): Promise | null> { + try { + const flows = await discoverFlows(args); + return dedupAndDetectDuplicates(flows, args.logger); + } catch (err: any) { + args.logger.warn(`buildFlowNameToPathMap failed unexpectedly: ${err?.message ?? String(err)}`); + return null; + } +} + +async function discoverFlows({ + inputFlowPaths, + projectRoot, + logger, +}: BuildFlowNameToPathMapArgs): Promise { + const realRoot = (await tryRealpath(projectRoot)) ?? projectRoot; + const fileLists = await Promise.all( + inputFlowPaths.map(async input => { + const abs = path.resolve(projectRoot, input); + const stat = await tryStat(abs); + if (!stat) { + logger.warn(`flow_paths entry "${input}" not found, skipping`); + return []; + } + if (stat.isFile() && YAML_EXT.test(abs)) { + return [abs]; + } + if (stat.isDirectory()) { + const out: string[] = []; + await walkDir(abs, new Set(), out, logger); + return out; + } + return []; + }) + ); + return Promise.all(fileLists.flat().map(absFile => makeEntry(absFile, realRoot))); +} + +async function makeEntry(absFile: string, realRoot: string): Promise { + const [name, realFile] = await Promise.all([ + readFlowName(absFile), + tryRealpath(absFile).then(rp => rp ?? absFile), + ]); + const rel = path.relative(realRoot, realFile); + const value = rel.startsWith('..') || path.isAbsolute(rel) ? absFile : rel; + return { name, path: value, realpath: realFile }; +} + +function dedupAndDetectDuplicates(flows: FlowEntry[], logger: bunyan): Map | null { + const byKey = new Map(); + for (const f of flows) { + if (!byKey.has(f.realpath)) { + byKey.set(f.realpath, f); + } + } + const unique = [...byKey.values()]; + + const nameToPath = new Map(); + const duplicates = new Map(); + for (const f of unique) { + if (nameToPath.has(f.name)) { + const existing = nameToPath.get(f.name)!; + const list = duplicates.get(f.name) ?? [existing]; + list.push(f.path); + duplicates.set(f.name, list); + } + nameToPath.set(f.name, f.path); + } + if (duplicates.size > 0) { + for (const [name, paths] of duplicates) { + logger.warn(`Duplicate Maestro flow name "${name}" across paths: ${paths.join(', ')}.`); + } + logger.warn( + 'Smart retry disabled for this run; will retry all flows on failure. ' + + 'Give each Maestro flow a unique name (file basename or top-level name) to enable smart retry.' + ); + return null; + } + return nameToPath; +} + +export async function walkDir( + dir: string, + visited: Set, + out: string[], + logger: bunyan +): Promise { + // Cycle protection: dedup on realpath (fall back to the original path when + // realpath fails so we still make progress on missing or restricted dirs). + const real = (await tryRealpath(dir)) ?? dir; + if (visited.has(real)) { + return; + } + visited.add(real); + + let entries; + try { + entries = await fs.readdir(dir, { withFileTypes: true }); + } catch (err: any) { + logger.warn(`readdir failed for ${dir}: ${err?.message ?? String(err)}`); + return; + } + entries.sort((a, b) => a.name.localeCompare(b.name)); + + for (const entry of entries) { + const full = path.join(dir, entry.name); + if (entry.isSymbolicLink()) { + const tgt = await tryStat(full); + if (!tgt) { + continue; + } + if (tgt.isDirectory()) { + await walkDir(full, visited, out, logger); + } else if (tgt.isFile() && isYamlFile(entry.name) && !isWorkspaceConfig(entry.name)) { + out.push(full); + } + } else if (entry.isDirectory()) { + await walkDir(full, visited, out, logger); + } else if (entry.isFile()) { + if (isYamlFile(entry.name) && !isWorkspaceConfig(entry.name)) { + out.push(full); + } + } + } +} + +export async function readFlowName(absFile: string): Promise { + const ext = path.extname(absFile); + const fallback = path.basename(absFile, ext); + let content: string; + try { + content = await fs.readFile(absFile, 'utf-8'); + } catch { + return fallback; + } + + let firstDoc; + try { + const docs = yaml.parseAllDocuments(content); + firstDoc = docs[0]; + } catch { + return fallback; + } + if (!firstDoc || firstDoc.errors.length > 0) { + return fallback; + } + + const parsed = firstDoc.toJS(); + const name = parsed?.name; + if (typeof name === 'string' && name.length > 0) { + return name; + } + return fallback; +} + +function isYamlFile(name: string): boolean { + return YAML_EXT.test(name); +} + +function isWorkspaceConfig(name: string): boolean { + return name.toLowerCase() === WORKSPACE_CONFIG_BASENAME; +} + +async function tryRealpath(p: string): Promise { + try { + return await fs.realpath(p); + } catch { + return null; + } +} + +async function tryStat(p: string): Promise<{ isFile(): boolean; isDirectory(): boolean } | null> { + try { + return await fs.stat(p); + } catch { + return null; + } +} diff --git a/packages/build-tools/src/steps/functions/maestroResultParser.ts b/packages/build-tools/src/steps/functions/maestroResultParser.ts index 620f962d35..9c321cb468 100644 --- a/packages/build-tools/src/steps/functions/maestroResultParser.ts +++ b/packages/build-tools/src/steps/functions/maestroResultParser.ts @@ -1,7 +1,6 @@ import { XMLBuilder, XMLParser, XMLValidator } from 'fast-xml-parser'; import fs from 'fs/promises'; import path from 'path'; -import { z } from 'zod'; export interface MaestroFlowResult { name: string; @@ -14,17 +13,9 @@ export interface MaestroFlowResult { properties: Record; } -// Maestro's TestDebugReporter creates timestamped directories, e.g. "2024-06-15_143022" -const TIMESTAMP_DIR_PATTERN = /^\d{4}-\d{2}-\d{2}_\d{6}$/; - // Per-attempt JUnit XML files use `*-attempt-N.xml` names; this extracts N. const ATTEMPT_PATTERN = /attempt-(\d+)/; -export function extractFlowKey(filename: string, prefix: string): string | null { - const match = filename.match(new RegExp(`^${prefix}-(.+)\\.json$`)); - return match?.[1] ?? null; -} - export interface JUnitTestCaseResult { name: string; status: 'passed' | 'failed'; @@ -140,42 +131,10 @@ export async function parseJUnitTestCases(junitDirectory: string): Promise; - -/** - * Parses an `ai-*.json` file produced by Maestro's TestDebugReporter. - * - * The file contains: - * - `flow_name`: derived from the YAML `config.name` field if present, otherwise - * the flow filename without extension. - * See: https://github.com/mobile-dev-inc/Maestro/blob/c0e95fd/maestro-cli/src/main/java/maestro/cli/runner/TestRunner.kt#L70 - * - `flow_file_path`: absolute path to the original flow YAML file. - * - `outputs`: screenshot defect data (unused here). - * - * Filename format: `ai-(flowName).json` where `/` in flowName is replaced with `_`. - * See: https://github.com/mobile-dev-inc/Maestro/blob/c0e95fd/maestro-cli/src/main/java/maestro/cli/report/TestDebugReporter.kt#L67 - */ -export async function parseFlowMetadata(filePath: string): Promise { - try { - const content = await fs.readFile(filePath, 'utf-8'); - const data = JSON.parse(content); - return FlowMetadataFileSchema.parse(data); - } catch { - return null; - } -} - export async function parseMaestroResults( junitDirectory: string, - testsDirectory: string, - projectRoot: string + nameToPath: Map | null ): Promise { - // 1. Parse JUnit XML files, tracking which file each result came from let junitEntries: string[]; try { junitEntries = await fs.readdir(junitDirectory); @@ -204,51 +163,8 @@ export async function parseMaestroResults( return []; } - // 2. Parse ai-*.json from debug output for flow_file_path + retryCount - const flowPathMap = new Map(); // flowName → flowFilePath - const flowOccurrences = new Map(); // flowName → count - - let entries: string[]; - try { - entries = await fs.readdir(testsDirectory); - } catch { - entries = []; - } - - const timestampDirs = entries.filter(name => TIMESTAMP_DIR_PATTERN.test(name)).sort(); - - for (const dir of timestampDirs) { - const dirPath = path.join(testsDirectory, dir); - let files: string[]; - try { - files = await fs.readdir(dirPath); - } catch { - continue; - } - - for (const file of files) { - const flowKey = extractFlowKey(file, 'ai'); - if (!flowKey) { - continue; - } - - const metadata = await parseFlowMetadata(path.join(dirPath, file)); - if (!metadata) { - continue; - } - - // Track latest path (last timestamp dir wins) - flowPathMap.set(metadata.flow_name, metadata.flow_file_path); - - // Count occurrences for retryCount - flowOccurrences.set(metadata.flow_name, (flowOccurrences.get(metadata.flow_name) ?? 0) + 1); - } - } - - // 3. Merge: JUnit results + ai-*.json metadata const results: MaestroFlowResult[] = []; - // Group results by flow name const resultsByName = new Map(); for (const entry of junitResultsWithSource) { const group = resultsByName.get(entry.result.name) ?? []; @@ -257,50 +173,28 @@ export async function parseMaestroResults( } for (const [flowName, flowEntries] of resultsByName) { - const flowFilePath = flowPathMap.get(flowName); - const relativePath = flowFilePath - ? await relativizePathAsync(flowFilePath, projectRoot) - : flowName; - - if (flowEntries.length === 1) { - // Single result for this flow — use ai-*.json occurrence count for retryCount - // (backward compat with old-style single JUnit file that gets overwritten) - const { result } = flowEntries[0]; - const occurrences = flowOccurrences.get(flowName) ?? 0; - const retryCount = Math.max(0, occurrences - 1); - + const flowPath = nameToPath?.get(flowName) ?? flowName; + // retryCount is derived from each entry's source filename (`attempt-N`); + // a single entry from `report.xml` (no marker) collapses to attempt 0. + const sorted = flowEntries + .map(entry => { + const match = entry.sourceFile.match(ATTEMPT_PATTERN); + const attemptIndex = match ? parseInt(match[1], 10) : 0; + return { ...entry, attemptIndex }; + }) + .sort((a, b) => a.attemptIndex - b.attemptIndex); + + for (const { result, attemptIndex } of sorted) { results.push({ name: flowName, - path: relativePath, + path: flowPath, status: result.status, errorMessage: result.errorMessage, duration: result.duration, - retryCount, + retryCount: attemptIndex, tags: result.tags, properties: result.properties, }); - } else { - // Multiple results — per-attempt JUnit files. Sort by attempt index from filename. - const sorted = flowEntries - .map(entry => { - const match = entry.sourceFile.match(ATTEMPT_PATTERN); - const attemptIndex = match ? parseInt(match[1], 10) : 0; - return { ...entry, attemptIndex }; - }) - .sort((a, b) => a.attemptIndex - b.attemptIndex); - - for (const { result, attemptIndex } of sorted) { - results.push({ - name: flowName, - path: relativePath, - status: result.status, - errorMessage: result.errorMessage, - duration: result.duration, - retryCount: attemptIndex, - tags: result.tags, - properties: result.properties, - }); - } } } @@ -312,15 +206,13 @@ export async function parseMaestroResults( * attempt's JUnit file, or `null` when the result cannot be trusted (caller * then falls back to dumb retry — re-run everything). * - * Mapping: only carries `name`, so we recover `flow_file_path` - * from `ai-${flow_name}.json` under testsDirectory and match it back to - * inputFlowPaths. + * Caller passes a precomputed `nameToPath` map built from the user's + * `inputFlowPaths`. We translate failed testcase names to flow paths via + * that map; unknown names → null. */ export async function parseFailedFlowsFromJUnit(args: { junitFile: string; - testsDirectory: string; - inputFlowPaths: string[]; - projectRoot: string; + nameToPath: Map; }): Promise { // fast-xml-parser is lenient — truncated XML can produce a partial parse // with some testcases dropped. Trusting that subset for smart retry would @@ -347,8 +239,9 @@ export async function parseFailedFlowsFromJUnit(args: { } // Two testcases with the same name (pass+fail or fail+fail) make it - // impossible to map back to a single input flow_path, since the - // ai-*.json keyed map collapses duplicates. Signal "unknown" → dumb retry. + // impossible to map back to a single input flow_path. Signal "unknown". + // Complementary to the map-build duplicate check (which catches the case + // where tag filter hides duplication from JUnit). const allNameCounts = new Map(); for (const tc of testcases) { allNameCounts.set(tc.name, (allNameCounts.get(tc.name) ?? 0) + 1); @@ -359,62 +252,17 @@ export async function parseFailedFlowsFromJUnit(args: { } } - // Build flow_name → flow_file_path map from ai-*.json across timestamped - // subdirectories (same traversal as parseMaestroResults). - const nameToPath = new Map(); - let entries: string[]; - try { - entries = await fs.readdir(args.testsDirectory); - } catch { - entries = []; - } - const timestampDirs = entries.filter(name => TIMESTAMP_DIR_PATTERN.test(name)).sort(); - for (const dir of timestampDirs) { - const dirPath = path.join(args.testsDirectory, dir); - let files: string[]; - try { - files = await fs.readdir(dirPath); - } catch { - continue; - } - for (const file of files) { - const flowKey = extractFlowKey(file, 'ai'); - if (!flowKey) { - continue; - } - const metadata = await parseFlowMetadata(path.join(dirPath, file)); - if (!metadata) { - continue; - } - // Latest timestamp dir wins if the same flow appears in multiple attempts. - nameToPath.set(metadata.flow_name, metadata.flow_file_path); - } - } - const matched: string[] = []; for (const tc of failing) { - const abs = nameToPath.get(tc.name); - if (!abs) { - return null; // unknown mapping; safer to fall back - } - const relative = await relativizePathAsync(abs, args.projectRoot); - // Accept exact matches and flow files discovered under an input directory - // (documented usage: `flow_path: ./maestro/flows` discovers nested .yml). - // Anything outside every input is treated as out-of-scope → dumb retry. - if (!args.inputFlowPaths.some(input => isPathWithinOrEqual(relative, input))) { + const p = args.nameToPath.get(tc.name); + if (p === undefined) { return null; } - matched.push(relative); + matched.push(p); } - return matched; } -function isPathWithinOrEqual(child: string, parent: string): boolean { - const rel = path.relative(parent, child); - return rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel)); -} - /** * Reads every *.xml in sourceDir (per-attempt JUnit files), picks the latest * attempt's per unique flow name (latest determined from the @@ -581,25 +429,3 @@ export async function copyLatestAttemptXml(args: { await fs.copyFile(path.join(args.sourceDir, winner), args.outputPath); } - -async function relativizePathAsync(flowFilePath: string, projectRoot: string): Promise { - if (!path.isAbsolute(flowFilePath)) { - return flowFilePath; - } - - // Resolve symlinks (e.g., /tmp -> /private/tmp on macOS) for consistent comparison - let resolvedRoot = projectRoot; - let resolvedFlow = flowFilePath; - try { - resolvedRoot = await fs.realpath(projectRoot); - } catch {} - try { - resolvedFlow = await fs.realpath(flowFilePath); - } catch {} - - const relative = path.relative(resolvedRoot, resolvedFlow); - if (relative.startsWith('..')) { - return flowFilePath; - } - return relative; -} diff --git a/packages/build-tools/src/steps/functions/maestroTests.ts b/packages/build-tools/src/steps/functions/maestroTests.ts index 4ce6493019..2fd408c021 100644 --- a/packages/build-tools/src/steps/functions/maestroTests.ts +++ b/packages/build-tools/src/steps/functions/maestroTests.ts @@ -11,6 +11,7 @@ import fs from 'fs/promises'; import path from 'path'; import { z } from 'zod'; +import { buildFlowNameToPathMap } from './maestroFlowDiscovery'; import { copyLatestAttemptXml, mergeJUnitReports, @@ -192,6 +193,14 @@ export function createMaestroTestsBuildFunction(): BuildFunction { throw new SystemError('Failed to create JUnit report directory', { cause: err }); } + // null → duplicate flow names; smart retry disabled, fall through to + // dumb retry (re-run everything) on failure. + const nameToPath = await buildFlowNameToPathMap({ + inputFlowPaths: flowPaths, + projectRoot: stepCtx.workingDirectory, + logger, + }); + // Retry loop. spawn-async error shapes: // ENOENT/EACCES → infra (binary missing/not executable) → SystemError. // numeric err.status → maestro exited non-zero → retry. @@ -203,6 +212,7 @@ export function createMaestroTestsBuildFunction(): BuildFunction { let flowsToRun: string[] = flowPaths; let lastAttemptExitCode: number | null = null; + const totalAttempts = retries + 1; for (let attempt = 0; attempt <= retries; attempt++) { const outputPath = outputFormat === 'junit' @@ -211,19 +221,25 @@ export function createMaestroTestsBuildFunction(): BuildFunction { ? path.join(testsDirectory, `${platform}-maestro-${outputFormat}.${outputFormat}`) : null; + const maestroArgs = buildMaestroArgs({ + flow_path: flowsToRun, + outputPath, + output_format: outputFormat, + shards, + include_tags: includeTags, + exclude_tags: excludeTags, + }); + logger.info( + `Running maestro (attempt ${attempt + 1}/${totalAttempts}): maestro ${maestroArgs.join(' ')}` + ); + try { - await spawn( - 'maestro', - buildMaestroArgs({ - flow_path: flowsToRun, - outputPath, - output_format: outputFormat, - shards, - include_tags: includeTags, - exclude_tags: excludeTags, - }), - { cwd: stepCtx.workingDirectory, env: spawnEnv, logger, signal } - ); + await spawn('maestro', maestroArgs, { + cwd: stepCtx.workingDirectory, + env: spawnEnv, + logger, + signal, + }); lastAttemptExitCode = 0; } catch (err: any) { if (err && (err.code === 'ENOENT' || err.code === 'EACCES')) { @@ -240,19 +256,25 @@ export function createMaestroTestsBuildFunction(): BuildFunction { break; } - if (outputFormat === 'junit' && outputPath) { + if (outputFormat === 'junit' && outputPath && nameToPath) { const failed = await parseFailedFlowsFromJUnit({ junitFile: outputPath, - testsDirectory, - inputFlowPaths: flowsToRun, - projectRoot: stepCtx.workingDirectory, + nameToPath, }); if (failed !== null && failed.length > 0) { flowsToRun = failed; + logger.info( + `Test failed; retrying ${failed.length} failed flow(s): ${failed.join(', ')}` + ); + } else { + flowsToRun = flowPaths; + logger.info('Test failed; could not determine failed subset, retrying all flows'); } + } else { + flowsToRun = flowPaths; + logger.info('Test failed, retrying all flows'); } - logger.info('Test failed, retrying...'); await sleepAsync(2000); } @@ -291,7 +313,6 @@ export function createMaestroTestsBuildFunction(): BuildFunction { // or throw (infra). A non-null non-zero status means the user's tests // failed every attempt. if (lastAttemptExitCode !== 0) { - const totalAttempts = retries + 1; throw new UserError( 'ERR_MAESTRO_TESTS_FAILED', `Maestro tests failed after ${totalAttempts} attempt${totalAttempts === 1 ? '' : 's'}.` diff --git a/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts b/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts index 6d7ec783f3..6a78a8f71f 100644 --- a/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts +++ b/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts @@ -1,6 +1,7 @@ import { BuildFunction, BuildStepInput, BuildStepInputValueTypeName } from '@expo/steps'; import { graphql } from 'gql.tada'; +import { buildFlowNameToPathMap } from './maestroFlowDiscovery'; import { MaestroFlowResult, parseMaestroResults } from './maestroResultParser'; import { CustomBuildContext } from '../../customBuildContext'; @@ -38,6 +39,11 @@ export function createReportMaestroTestResultsFunction(ctx: CustomBuildContext): allowedValueTypeName: BuildStepInputValueTypeName.STRING, defaultValue: '${{ env.HOME }}/.maestro/tests', }), + BuildStepInput.createProvider({ + id: 'flow_paths', + required: false, + allowedValueTypeName: BuildStepInputValueTypeName.JSON, + }), ], fn: async (stepsCtx, { inputs }) => { const { logger } = stepsCtx; @@ -51,14 +57,29 @@ export function createReportMaestroTestResultsFunction(ctx: CustomBuildContext): logger.info('No JUnit directory provided, skipping test results report'); return; } - const testsDirectory = inputs.tests_directory.value as string; - try { - const flowResults = await parseMaestroResults( - junitDirectory, - testsDirectory, - stepsCtx.workingDirectory + const flowPathsRaw = inputs.flow_paths.value as unknown; + let nameToPath: Map | null = null; + if ( + Array.isArray(flowPathsRaw) && + flowPathsRaw.length > 0 && + flowPathsRaw.every(p => typeof p === 'string' && p.length > 0) + ) { + nameToPath = await buildFlowNameToPathMap({ + inputFlowPaths: flowPathsRaw, + projectRoot: stepsCtx.workingDirectory, + logger, + }); + } else if (flowPathsRaw !== undefined) { + logger.warn( + 'Ignoring malformed flow_paths input (expected a non-empty array of non-empty strings).' ); + } + // flow_paths absent (undefined) — silent fallback for backward compat + // with universe deployments that don't yet pass this input. + + try { + const flowResults = await parseMaestroResults(junitDirectory, nameToPath); if (flowResults.length === 0) { logger.info('No maestro test results found, skipping report'); return; From 7f3c9ceab453d386e8445a52673e78c0206c1b73 Mon Sep 17 00:00:00 2001 From: Ash Wu Date: Thu, 30 Apr 2026 16:25:28 +0800 Subject: [PATCH 2/3] fix param name & use zod according to previous comments Signed-off-by: Ash Wu --- .../reportMaestroTestResults.test.ts | 2 +- .../steps/functions/maestroFlowDiscovery.ts | 2 +- .../functions/reportMaestroTestResults.ts | 36 ++++++++++--------- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/packages/build-tools/src/steps/functions/__tests__/reportMaestroTestResults.test.ts b/packages/build-tools/src/steps/functions/__tests__/reportMaestroTestResults.test.ts index 908fa38c51..d37491d30f 100644 --- a/packages/build-tools/src/steps/functions/__tests__/reportMaestroTestResults.test.ts +++ b/packages/build-tools/src/steps/functions/__tests__/reportMaestroTestResults.test.ts @@ -85,7 +85,7 @@ describe(createReportMaestroTestResultsFunction, () => { await createStep({ callInputs: { - flow_paths: ['.maestro/home.yml'], + flow_path: ['.maestro/home.yml'], }, }).executeAsync(); diff --git a/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts b/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts index c3f101820a..ac8fa952a6 100644 --- a/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts +++ b/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts @@ -41,7 +41,7 @@ async function discoverFlows({ const abs = path.resolve(projectRoot, input); const stat = await tryStat(abs); if (!stat) { - logger.warn(`flow_paths entry "${input}" not found, skipping`); + logger.warn(`flow_path entry "${input}" not found, skipping`); return []; } if (stat.isFile() && YAML_EXT.test(abs)) { diff --git a/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts b/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts index 6a78a8f71f..c33eb94de7 100644 --- a/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts +++ b/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts @@ -1,10 +1,13 @@ import { BuildFunction, BuildStepInput, BuildStepInputValueTypeName } from '@expo/steps'; import { graphql } from 'gql.tada'; +import { z } from 'zod'; import { buildFlowNameToPathMap } from './maestroFlowDiscovery'; import { MaestroFlowResult, parseMaestroResults } from './maestroResultParser'; import { CustomBuildContext } from '../../customBuildContext'; +const FlowPathSchema = z.array(z.string().min(1)).min(1); + const CREATE_MUTATION = graphql(` mutation CreateWorkflowDeviceTestCaseResults($input: CreateWorkflowDeviceTestCaseResultsInput!) { workflowDeviceTestCaseResult { @@ -40,7 +43,7 @@ export function createReportMaestroTestResultsFunction(ctx: CustomBuildContext): defaultValue: '${{ env.HOME }}/.maestro/tests', }), BuildStepInput.createProvider({ - id: 'flow_paths', + id: 'flow_path', required: false, allowedValueTypeName: BuildStepInputValueTypeName.JSON, }), @@ -58,24 +61,23 @@ export function createReportMaestroTestResultsFunction(ctx: CustomBuildContext): return; } - const flowPathsRaw = inputs.flow_paths.value as unknown; + const flowPathRaw = inputs.flow_path.value; let nameToPath: Map | null = null; - if ( - Array.isArray(flowPathsRaw) && - flowPathsRaw.length > 0 && - flowPathsRaw.every(p => typeof p === 'string' && p.length > 0) - ) { - nameToPath = await buildFlowNameToPathMap({ - inputFlowPaths: flowPathsRaw, - projectRoot: stepsCtx.workingDirectory, - logger, - }); - } else if (flowPathsRaw !== undefined) { - logger.warn( - 'Ignoring malformed flow_paths input (expected a non-empty array of non-empty strings).' - ); + if (flowPathRaw !== undefined) { + const parsed = FlowPathSchema.safeParse(flowPathRaw); + if (parsed.success) { + nameToPath = await buildFlowNameToPathMap({ + inputFlowPaths: parsed.data, + projectRoot: stepsCtx.workingDirectory, + logger, + }); + } else { + logger.warn( + 'Ignoring malformed flow_path input (expected a non-empty array of non-empty strings).' + ); + } } - // flow_paths absent (undefined) — silent fallback for backward compat + // flow_path absent (undefined) — silent fallback for backward compat // with universe deployments that don't yet pass this input. try { From 075567845dccdd19173cef0439087cf3a08f3a44 Mon Sep 17 00:00:00 2001 From: Ash Wu Date: Tue, 5 May 2026 20:53:10 +0800 Subject: [PATCH 3/3] address comments Signed-off-by: Ash Wu --- .../build-tools/src/__mocks__/fs/promises.ts | 5 +-- .../steps/functions/maestroFlowDiscovery.ts | 38 +++++++------------ .../functions/reportMaestroTestResults.ts | 2 - 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/packages/build-tools/src/__mocks__/fs/promises.ts b/packages/build-tools/src/__mocks__/fs/promises.ts index 1f94062b12..4f076034e3 100644 --- a/packages/build-tools/src/__mocks__/fs/promises.ts +++ b/packages/build-tools/src/__mocks__/fs/promises.ts @@ -1,10 +1,7 @@ import { fs } from 'memfs'; -const fsRealpath = fs.realpath; -(fsRealpath as any).native = fsRealpath; - const fsRm = (path: string, options: object): Promise => { return fs.promises.rm(path, options); }; -module.exports = { ...fs.promises, realpath: fsRealpath, rm: fsRm }; +module.exports = { ...fs.promises, rm: fsRm }; diff --git a/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts b/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts index ac8fa952a6..6df229c416 100644 --- a/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts +++ b/packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts @@ -1,4 +1,5 @@ import type { bunyan } from '@expo/logger'; +import { asyncResult } from '@expo/results'; import fs from 'fs/promises'; import path from 'path'; import yaml from 'yaml'; @@ -35,15 +36,17 @@ async function discoverFlows({ projectRoot, logger, }: BuildFlowNameToPathMapArgs): Promise { - const realRoot = (await tryRealpath(projectRoot)) ?? projectRoot; + const realRootResult = await asyncResult(fs.realpath(projectRoot)); + const realRoot = realRootResult.ok ? realRootResult.value : projectRoot; const fileLists = await Promise.all( inputFlowPaths.map(async input => { const abs = path.resolve(projectRoot, input); - const stat = await tryStat(abs); - if (!stat) { + const statResult = await asyncResult(fs.stat(abs)); + if (!statResult.ok) { logger.warn(`flow_path entry "${input}" not found, skipping`); return []; } + const stat = statResult.value; if (stat.isFile() && YAML_EXT.test(abs)) { return [abs]; } @@ -59,10 +62,11 @@ async function discoverFlows({ } async function makeEntry(absFile: string, realRoot: string): Promise { - const [name, realFile] = await Promise.all([ + const [name, realFileResult] = await Promise.all([ readFlowName(absFile), - tryRealpath(absFile).then(rp => rp ?? absFile), + asyncResult(fs.realpath(absFile)), ]); + const realFile = realFileResult.ok ? realFileResult.value : absFile; const rel = path.relative(realRoot, realFile); const value = rel.startsWith('..') || path.isAbsolute(rel) ? absFile : rel; return { name, path: value, realpath: realFile }; @@ -109,7 +113,8 @@ export async function walkDir( ): Promise { // Cycle protection: dedup on realpath (fall back to the original path when // realpath fails so we still make progress on missing or restricted dirs). - const real = (await tryRealpath(dir)) ?? dir; + const realResult = await asyncResult(fs.realpath(dir)); + const real = realResult.ok ? realResult.value : dir; if (visited.has(real)) { return; } @@ -127,10 +132,11 @@ export async function walkDir( for (const entry of entries) { const full = path.join(dir, entry.name); if (entry.isSymbolicLink()) { - const tgt = await tryStat(full); - if (!tgt) { + const tgtResult = await asyncResult(fs.stat(full)); + if (!tgtResult.ok) { continue; } + const tgt = tgtResult.value; if (tgt.isDirectory()) { await walkDir(full, visited, out, logger); } else if (tgt.isFile() && isYamlFile(entry.name) && !isWorkspaceConfig(entry.name)) { @@ -182,19 +188,3 @@ function isYamlFile(name: string): boolean { function isWorkspaceConfig(name: string): boolean { return name.toLowerCase() === WORKSPACE_CONFIG_BASENAME; } - -async function tryRealpath(p: string): Promise { - try { - return await fs.realpath(p); - } catch { - return null; - } -} - -async function tryStat(p: string): Promise<{ isFile(): boolean; isDirectory(): boolean } | null> { - try { - return await fs.stat(p); - } catch { - return null; - } -} diff --git a/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts b/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts index c33eb94de7..46a0a0d191 100644 --- a/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts +++ b/packages/build-tools/src/steps/functions/reportMaestroTestResults.ts @@ -77,8 +77,6 @@ export function createReportMaestroTestResultsFunction(ctx: CustomBuildContext): ); } } - // flow_path absent (undefined) — silent fallback for backward compat - // with universe deployments that don't yet pass this input. try { const flowResults = await parseMaestroResults(junitDirectory, nameToPath);