address comments

Signed-off-by: Ash Wu <hsatac@gmail.com>

Author: hSATAC

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

@@ -1075,18 +1075,17 @@ describe('mergeJUnitReports', () => {
     ).rejects.toThrow(/no \*\.xml files/);
   });
 
-  it('throws when no parseable testcases are found across inputs', async () => {
-    // Malformed XML with no parseable <testsuite> / <testcase>. Without a
-    // throw here, mergeJUnitReports would silently emit an empty merged
-    // document and Phase 3's copyLatestAttemptXml fallback (which only
-    // triggers on throw) would never run.
+  it('throws on invalid XML so caller can fall back to copyLatestAttemptXml', async () => {
+    // Without a throw, mergeJUnitReports would silently emit an empty merged
+    // document and the copy-latest fallback (which only triggers on throw)
+    // would never run.
     vol.fromJSON({
       '/tmp/r/android-maestro-junit-attempt-0.xml': 'not xml at all',
     });
 
     await expect(
       mergeJUnitReports({ sourceDir: '/tmp/r', outputPath: '/tmp/final.xml' })
-    ).rejects.toThrow(/no parseable testcases/);
+    ).rejects.toThrow(/invalid XML/);
   });
 
   it('throws when every input has <testsuites> but no <testcase> elements', async () => {

packages/build-tools/src/steps/functions/maestroResultParser.ts

@@ -449,23 +449,19 @@ export async function mergeJUnitReports(args: {
     }))
   );
   const fileGroups: FileGroup[] = [];
-  const skippedFiles: string[] = [];
   for (const { filename, content } of contents) {
     if (XMLValidator.validate(content) !== true) {
-      skippedFiles.push(filename);
-      continue;
+      throw new Error(`mergeJUnitReports: invalid XML in ${filename}`);
     }
     let parsed: any;
     try {
       parsed = xmlParser.parse(content);
-    } catch {
-      skippedFiles.push(filename);
-      continue;
+    } catch (err) {
+      throw new Error(`mergeJUnitReports: failed to parse ${filename}`, { cause: err });
     }
     const testsuites = parsed?.testsuites?.testsuite;
     if (!Array.isArray(testsuites)) {
-      skippedFiles.push(filename);
-      continue;
+      throw new Error(`mergeJUnitReports: no <testsuite> array in ${filename}`);
     }
     const match = filename.match(ATTEMPT_PATTERN);
     const attemptIndex = match ? parseInt(match[1], 10) : 0;
@@ -486,18 +482,11 @@ export async function mergeJUnitReports(args: {
       }
     }
     if (testcasesByName.size === 0) {
-      skippedFiles.push(filename);
-      continue;
+      throw new Error(`mergeJUnitReports: no parseable testcases in ${filename}`);
     }
     fileGroups.push({ attemptIndex, filename, content, testcasesByName });
   }
 
-  if (skippedFiles.length > 0) {
-    throw new Error(
-      `mergeJUnitReports: no parseable testcases found in ${skippedFiles.join(', ')}`
-    );
-  }
-
   // Single attempt: copy the original XML so suite-level metadata (testsuite
   // attributes, <system-out>, etc.) survives. The rebuild path below would
   // collapse those to a single attribute-less <testsuite>.