Skip to content

Commit 2c32bf1

Browse files
hSATAChSATAC
committed
[build-tools] Add smart retry to eas/run_maestro_tests
In `reuse_devices` mode, narrow the flow_paths to only the failing flows on retry. After a failed attempt, parse the per-attempt JUnit XML to identify the flows that failed and pass just those to the next maestro invocation. Falls back to retrying everything when the JUnit cannot be trusted (missing testcases, conflicting names, malformed XML). Replaces copy-latest with mergeJUnitReports — picks the latest attempt's result per flow across retries — so the merged final report reflects the best outcome each flow achieved. Falls back to copy-latest on data errors. Signed-off-by: Ash Wu <hsatac@gmail.com>
1 parent 0ccb966 commit 2c32bf1

4 files changed

Lines changed: 779 additions & 53 deletions

File tree

‎packages/build-tools/src/steps/functions/__tests__/maestroResultParser.test.ts‎

Lines changed: 361 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
import { XMLParser } from 'fast-xml-parser';
12
import fs from 'fs/promises';
23
import { vol } from 'memfs';
34

45
import {
56
copyLatestAttemptXml,
7+
mergeJUnitReports,
8+
parseFailedFlowsFromJUnit,
69
parseFlowMetadata,
710
parseJUnitTestCases,
811
parseMaestroResults,
@@ -806,6 +809,364 @@ describe(parseJUnitTestCases, () => {
806809
});
807810
});
808811

812+
describe('parseFailedFlowsFromJUnit', () => {
813+
it('returns the subset of input flow paths whose testcases failed', async () => {
814+
// memfs setup: 3 input flows, 1 junit file with 1 failure, 1 timestamp dir with 3 ai-*.json
815+
vol.fromJSON({
816+
'/project/flows/login.yaml': '',
817+
'/project/flows/search.yaml': '',
818+
'/project/flows/checkout.yaml': '',
819+
'/tmp/junit-reports/android-maestro-junit-attempt-0.xml': `<?xml version="1.0"?>
820+
<testsuites>
821+
<testsuite>
822+
<testcase name="Login" status="SUCCESS" time="1.0" />
823+
<testcase name="Search" status="SUCCESS" time="1.0" />
824+
<testcase name="Checkout" time="1.0"><failure>something</failure></testcase>
825+
</testsuite>
826+
</testsuites>`,
827+
'/tmp/tests/2026-04-23_120000/ai-Login.json': JSON.stringify({
828+
flow_name: 'Login',
829+
flow_file_path: '/project/flows/login.yaml',
830+
}),
831+
'/tmp/tests/2026-04-23_120000/ai-Search.json': JSON.stringify({
832+
flow_name: 'Search',
833+
flow_file_path: '/project/flows/search.yaml',
834+
}),
835+
'/tmp/tests/2026-04-23_120000/ai-Checkout.json': JSON.stringify({
836+
flow_name: 'Checkout',
837+
flow_file_path: '/project/flows/checkout.yaml',
838+
}),
839+
});
840+
841+
const result = await parseFailedFlowsFromJUnit({
842+
junitFile: '/tmp/junit-reports/android-maestro-junit-attempt-0.xml',
843+
testsDirectory: '/tmp/tests',
844+
inputFlowPaths: ['flows/login.yaml', 'flows/search.yaml', 'flows/checkout.yaml'],
845+
projectRoot: '/project',
846+
});
847+
848+
expect(result).toEqual(['flows/checkout.yaml']);
849+
});
850+
851+
it('accepts flow files discovered under a directory input (documented usage)', async () => {
852+
// Users may set `flow_path: ./maestro/flows` (a directory). Maestro then
853+
// discovers the YAMLs inside; smart retry must still subset to the failing
854+
// child file, not fall back to dumb retry.
855+
vol.fromJSON({
856+
'/project/flows/login.yaml': '',
857+
'/project/flows/checkout.yaml': '',
858+
'/tmp/junit-reports/attempt-0.xml': `<?xml version="1.0"?>
859+
<testsuites><testsuite>
860+
<testcase name="Login" status="SUCCESS" time="1.0" />
861+
<testcase name="Checkout" time="1.0"><failure>x</failure></testcase>
862+
</testsuite></testsuites>`,
863+
'/tmp/tests/2026-04-23_120000/ai-Login.json': JSON.stringify({
864+
flow_name: 'Login',
865+
flow_file_path: '/project/flows/login.yaml',
866+
}),
867+
'/tmp/tests/2026-04-23_120000/ai-Checkout.json': JSON.stringify({
868+
flow_name: 'Checkout',
869+
flow_file_path: '/project/flows/checkout.yaml',
870+
}),
871+
});
872+
873+
const result = await parseFailedFlowsFromJUnit({
874+
junitFile: '/tmp/junit-reports/attempt-0.xml',
875+
testsDirectory: '/tmp/tests',
876+
inputFlowPaths: ['flows'],
877+
projectRoot: '/project',
878+
});
879+
880+
expect(result).toEqual(['flows/checkout.yaml']);
881+
});
882+
883+
it('returns null when two failing testcases share the same name (collision)', async () => {
884+
vol.fromJSON({
885+
'/project/flows/a.yaml': '',
886+
'/project/flows/b.yaml': '',
887+
'/tmp/junit-reports/android-maestro-junit-attempt-0.xml': `<?xml version="1.0"?>
888+
<testsuites><testsuite>
889+
<testcase name="Duplicate" time="1.0"><failure>x</failure></testcase>
890+
<testcase name="Duplicate" time="1.0"><failure>y</failure></testcase>
891+
</testsuite></testsuites>`,
892+
'/tmp/tests/2026-04-23_120000/ai-Duplicate.json': JSON.stringify({
893+
flow_name: 'Duplicate',
894+
flow_file_path: '/project/flows/a.yaml',
895+
}),
896+
});
897+
898+
const result = await parseFailedFlowsFromJUnit({
899+
junitFile: '/tmp/junit-reports/android-maestro-junit-attempt-0.xml',
900+
testsDirectory: '/tmp/tests',
901+
inputFlowPaths: ['flows/a.yaml', 'flows/b.yaml'],
902+
projectRoot: '/project',
903+
});
904+
905+
expect(result).toBeNull();
906+
});
907+
908+
it('returns null when a failing testcase shares a name with any other testcase (pass or fail)', async () => {
909+
vol.fromJSON({
910+
'/project/flows/a.yaml': '',
911+
'/project/flows/b.yaml': '',
912+
'/tmp/junit-reports/android-maestro-junit-attempt-0.xml': `<?xml version="1.0"?>
913+
<testsuites><testsuite>
914+
<testcase name="Shared" status="SUCCESS" time="1.0" />
915+
<testcase name="Shared" time="2.0"><failure>x</failure></testcase>
916+
</testsuite></testsuites>`,
917+
'/tmp/tests/2026-04-23_120000/ai-Shared.json': JSON.stringify({
918+
flow_name: 'Shared',
919+
flow_file_path: '/project/flows/a.yaml',
920+
}),
921+
});
922+
923+
const result = await parseFailedFlowsFromJUnit({
924+
junitFile: '/tmp/junit-reports/android-maestro-junit-attempt-0.xml',
925+
testsDirectory: '/tmp/tests',
926+
inputFlowPaths: ['flows/a.yaml', 'flows/b.yaml'],
927+
projectRoot: '/project',
928+
});
929+
930+
expect(result).toBeNull();
931+
});
932+
933+
it('returns null when junit file does not exist', async () => {
934+
vol.fromJSON({
935+
'/project/flows/a.yaml': '',
936+
});
937+
const result = await parseFailedFlowsFromJUnit({
938+
junitFile: '/tmp/missing.xml',
939+
testsDirectory: '/tmp/tests',
940+
inputFlowPaths: ['flows/a.yaml'],
941+
projectRoot: '/project',
942+
});
943+
expect(result).toBeNull();
944+
});
945+
946+
it('returns null when junit file is malformed', async () => {
947+
vol.fromJSON({
948+
'/tmp/junit-reports/bad.xml': 'this is not xml',
949+
});
950+
const result = await parseFailedFlowsFromJUnit({
951+
junitFile: '/tmp/junit-reports/bad.xml',
952+
testsDirectory: '/tmp/tests',
953+
inputFlowPaths: ['flows/a.yaml'],
954+
projectRoot: '/project',
955+
});
956+
expect(result).toBeNull();
957+
});
958+
959+
it('returns null when junit XML is truncated mid-tag (partial parse risk)', async () => {
960+
// fast-xml-parser can produce a partial parse from truncated XML — without
961+
// strict validation, smart retry would only retry the visible failures and
962+
// silently skip flows that were cut off, masking real failures when the
963+
// subset retry passes. Validation must reject the file → dumb retry.
964+
vol.fromJSON({
965+
'/project/flows/a.yaml': '',
966+
'/project/flows/b.yaml': '',
967+
'/tmp/junit-reports/attempt-0.xml': `<?xml version="1.0"?>
968+
<testsuites><testsuite>
969+
<testcase name="A" time="1.0"><failure>x</failure></testcase>
970+
<testcase name="B"`, // intentionally truncated mid-tag
971+
'/tmp/tests/2026-04-23_120000/ai-A.json': JSON.stringify({
972+
flow_name: 'A',
973+
flow_file_path: '/project/flows/a.yaml',
974+
}),
975+
'/tmp/tests/2026-04-23_120000/ai-B.json': JSON.stringify({
976+
flow_name: 'B',
977+
flow_file_path: '/project/flows/b.yaml',
978+
}),
979+
});
980+
const result = await parseFailedFlowsFromJUnit({
981+
junitFile: '/tmp/junit-reports/attempt-0.xml',
982+
testsDirectory: '/tmp/tests',
983+
inputFlowPaths: ['flows/a.yaml', 'flows/b.yaml'],
984+
projectRoot: '/project',
985+
});
986+
expect(result).toBeNull();
987+
});
988+
989+
it('returns null when junit XML has unclosed tags (partial parse risk)', async () => {
990+
vol.fromJSON({
991+
'/project/flows/a.yaml': '',
992+
'/tmp/junit-reports/attempt-0.xml': `<?xml version="1.0"?>
993+
<testsuites><testsuite>
994+
<testcase name="A" time="1.0"><failure>x</failure></testcase>
995+
</testsuite>`, // missing </testsuites>
996+
'/tmp/tests/2026-04-23_120000/ai-A.json': JSON.stringify({
997+
flow_name: 'A',
998+
flow_file_path: '/project/flows/a.yaml',
999+
}),
1000+
});
1001+
const result = await parseFailedFlowsFromJUnit({
1002+
junitFile: '/tmp/junit-reports/attempt-0.xml',
1003+
testsDirectory: '/tmp/tests',
1004+
inputFlowPaths: ['flows/a.yaml'],
1005+
projectRoot: '/project',
1006+
});
1007+
expect(result).toBeNull();
1008+
});
1009+
});
1010+
1011+
describe('mergeJUnitReports', () => {
1012+
it('identity-copies a single attempt (preserves suite-level metadata)', async () => {
1013+
// Single-attempt runs should land in final_report_path with the same
1014+
// suite-level attributes (tests/failures/time) and non-testcase children
1015+
// (e.g. <system-out>) that the legacy bash `cp` upload preserved. The
1016+
// rebuild path used for multi-attempt merges drops these, so the single
1017+
// case must short-circuit to a byte-equivalent copy.
1018+
const input = `<?xml version="1.0"?>
1019+
<testsuites>
1020+
<testsuite name="Maestro Flows" tests="2" failures="0" time="3.0" device="Pixel 7">
1021+
<testcase name="A" status="SUCCESS" time="1.0" />
1022+
<testcase name="B" status="SUCCESS" time="2.0" />
1023+
<system-out>boot complete</system-out>
1024+
</testsuite>
1025+
</testsuites>`;
1026+
vol.fromJSON({
1027+
'/tmp/junit-reports/android-maestro-junit-attempt-0.xml': input,
1028+
});
1029+
1030+
await mergeJUnitReports({
1031+
sourceDir: '/tmp/junit-reports',
1032+
outputPath: '/tmp/final.xml',
1033+
});
1034+
1035+
const out = await fs.readFile('/tmp/final.xml', 'utf-8');
1036+
expect(out).toBe(input);
1037+
});
1038+
1039+
it('keeps the latest attempt per flow name across multiple files', async () => {
1040+
vol.fromJSON({
1041+
'/tmp/r/android-maestro-junit-attempt-0.xml': `<?xml version="1.0"?>
1042+
<testsuites><testsuite>
1043+
<testcase name="A" status="SUCCESS" time="1.0" />
1044+
<testcase name="B" time="2.0"><failure>bad</failure></testcase>
1045+
</testsuite></testsuites>`,
1046+
'/tmp/r/android-maestro-junit-attempt-1.xml': `<?xml version="1.0"?>
1047+
<testsuites><testsuite>
1048+
<testcase name="B" status="SUCCESS" time="3.0" />
1049+
</testsuite></testsuites>`,
1050+
});
1051+
1052+
await mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' });
1053+
1054+
const out = await fs.readFile('/tmp/final.xml', 'utf-8');
1055+
const parsed = new XMLParser({ ignoreAttributes: false }).parse(out);
1056+
const testcases = parsed.testsuites.testsuite.testcase;
1057+
const a = testcases.find((t: any) => t['@_name'] === 'A');
1058+
const b = testcases.find((t: any) => t['@_name'] === 'B');
1059+
expect(a['@_status']).toBe('SUCCESS'); // from attempt 0
1060+
expect(b['@_status']).toBe('SUCCESS'); // from attempt 1 (latest)
1061+
expect(b.failure).toBeUndefined();
1062+
});
1063+
1064+
it('throws when source directory contains no *.xml files', async () => {
1065+
// No per-attempt JUnit files were ever written (maestro crashed before
1066+
// producing output). Phase 3 in runMaestroTests treats this as a SystemError
1067+
// — without a throw here, mergeJUnitReports would silently write an empty
1068+
// <testsuite> and the caller would upload a misleading empty report.
1069+
vol.fromJSON({
1070+
'/tmp/r/.gitkeep': '',
1071+
});
1072+
1073+
await expect(
1074+
mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' })
1075+
).rejects.toThrow(/no \*\.xml files/);
1076+
});
1077+
1078+
it('throws when no parseable testcases are found across inputs', async () => {
1079+
// Malformed XML with no parseable <testsuite> / <testcase>. Without a
1080+
// throw here, mergeJUnitReports would silently emit an empty merged
1081+
// document and Phase 3's copyLatestAttemptXml fallback (which only
1082+
// triggers on throw) would never run.
1083+
vol.fromJSON({
1084+
'/tmp/r/android-maestro-junit-attempt-0.xml': 'not xml at all',
1085+
});
1086+
1087+
await expect(
1088+
mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' })
1089+
).rejects.toThrow(/no parseable testcases/);
1090+
});
1091+
1092+
it('throws when every input has <testsuites> but no <testcase> elements', async () => {
1093+
vol.fromJSON({
1094+
'/tmp/r/android-maestro-junit-attempt-0.xml': `<?xml version="1.0"?>
1095+
<testsuites><testsuite></testsuite></testsuites>`,
1096+
});
1097+
1098+
await expect(
1099+
mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' })
1100+
).rejects.toThrow(/no parseable testcases/);
1101+
});
1102+
1103+
it('throws when any input XML is malformed (even if others parse)', async () => {
1104+
vol.fromJSON({
1105+
'/tmp/r/attempt-0.xml': `<?xml version="1.0"?><testsuites><testsuite><testcase name="A" status="SUCCESS" time="1.0"/></testsuite></testsuites>`,
1106+
'/tmp/r/attempt-1.xml': 'garbage not xml',
1107+
});
1108+
await expect(
1109+
mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' })
1110+
).rejects.toThrow();
1111+
});
1112+
1113+
it('throws when an input XML is truncated mid-tag (would otherwise partial-parse)', async () => {
1114+
// fast-xml-parser is lenient and can return partial results from truncated
1115+
// XML. Without strict validation, mergeJUnitReports would emit a merged
1116+
// report missing the cut-off flows and Phase 3's copy-latest fallback
1117+
// would never run. Validation must reject the file.
1118+
vol.fromJSON({
1119+
'/tmp/r/attempt-0.xml': `<?xml version="1.0"?>
1120+
<testsuites><testsuite>
1121+
<testcase name="A" status="SUCCESS" time="1.0"/>
1122+
<testcase name="B"`, // truncated mid-tag
1123+
});
1124+
await expect(
1125+
mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' })
1126+
).rejects.toThrow();
1127+
});
1128+
1129+
it('throws when an input XML has unclosed tags (would otherwise partial-parse)', async () => {
1130+
vol.fromJSON({
1131+
'/tmp/r/attempt-0.xml': `<?xml version="1.0"?>
1132+
<testsuites><testsuite>
1133+
<testcase name="A" status="SUCCESS" time="1.0"/>
1134+
</testsuite>`, // missing </testsuites>
1135+
});
1136+
await expect(
1137+
mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' })
1138+
).rejects.toThrow();
1139+
});
1140+
1141+
it('throws when any input XML parses but has no testcases (even if others have testcases)', async () => {
1142+
vol.fromJSON({
1143+
'/tmp/r/attempt-0.xml': `<?xml version="1.0"?><testsuites><testsuite><testcase name="A" status="SUCCESS" time="1.0"/></testsuite></testsuites>`,
1144+
'/tmp/r/attempt-1.xml': `<?xml version="1.0"?><testsuites></testsuites>`,
1145+
});
1146+
await expect(
1147+
mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' })
1148+
).rejects.toThrow();
1149+
});
1150+
1151+
it('preserves duplicate names within the same attempt', async () => {
1152+
vol.fromJSON({
1153+
'/tmp/r/attempt-0.xml': `<?xml version="1.0"?>
1154+
<testsuites><testsuite>
1155+
<testcase name="Dup" status="SUCCESS" time="1.0" />
1156+
<testcase name="Dup" status="SUCCESS" time="2.0" />
1157+
</testsuite></testsuites>`,
1158+
});
1159+
1160+
await mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' });
1161+
1162+
const out = await fs.readFile('/tmp/final.xml', 'utf-8');
1163+
const parsed = new XMLParser({ ignoreAttributes: false, isArray: n => n === 'testcase' }).parse(
1164+
out
1165+
);
1166+
expect(parsed.testsuites.testsuite.testcase).toHaveLength(2);
1167+
});
1168+
});
1169+
8091170
describe('copyLatestAttemptXml', () => {
8101171
it('picks the highest attempt number', async () => {
8111172
vol.fromJSON({

0 commit comments

Comments
 (0)