Fix stale api-runs data polluting test report archive counts

framework/testtools/src/main/java/org/apache/ofbiz/testtools/report/JUnitXmlCounter.java

@@ -43,6 +43,18 @@ public final class JUnitXmlCounter {
 
     private static final String MODULE = JUnitXmlCounter.class.getName();
 
+    /**
+     * Name of the directory {@code org.apache.ofbiz.testtools.TestRunServices} accumulates one
+     * subdirectory per API-triggered run under (each already archived independently, under its
+     * own runId). A gradle-triggered {@code test}/{@code testIntegration} run is handed the
+     * shared parent directory (e.g. {@code runtime/logs/test-results}) as its own resultsDir,
+     * which sits right next to this one - recursing into it here would keep summing every past
+     * API-triggered run's counts into every future gradle run's manifest too. {@link
+     * org.apache.ofbiz.testtools.report.TestReportArchiver#copyRecursive} skips it for the same
+     * reason when copying resultsDir into the archived run folder.
+     */
+    static final String API_RUNS_DIR_NAME = "api-runs";
+
     private JUnitXmlCounter() {
     }
 
@@ -106,6 +118,9 @@ private static List<File> listXmlFilesRecursively(File dir) {
         }
         for (File child : children) {
             if (child.isDirectory()) {
+                if (API_RUNS_DIR_NAME.equals(child.getName())) {
+                    continue;
+                }
                 result.addAll(listXmlFilesRecursively(child));
             } else if (child.getName().endsWith(".xml")) {
                 result.add(child);

framework/testtools/src/main/java/org/apache/ofbiz/testtools/report/TestReportArchiver.java

@@ -119,6 +119,15 @@ private static void copyRecursive(Path source, Path dest) throws IOException {
             Files.createDirectories(dest);
             try (var children = Files.list(source)) {
                 for (Path child : (Iterable<Path>) children::iterator) {
+                    // See JUnitXmlCounter.API_RUNS_DIR_NAME's javadoc: api-runs/ holds every past
+                    // API-triggered run's own results, already archived independently under its
+                    // own runId. Copying it into every gradle-triggered archive too would
+                    // duplicate that entire (ever-growing) history into results/ on every single
+                    // test/testIntegration run, on top of corrupting the counts JUnitXmlCounter
+                    // sums from this same tree.
+                    if (JUnitXmlCounter.API_RUNS_DIR_NAME.equals(child.getFileName().toString())) {
+                        continue;
+                    }
                     copyRecursive(child, dest.resolve(child.getFileName().toString()));
                 }
             }

framework/testtools/src/test/java/org/apache/ofbiz/testtools/report/JUnitXmlCounterTest.java

@@ -59,6 +59,24 @@ void skipsUnparsableFilesInsteadOfThrowing(@TempDir File resultsDir) throws IOEx
         assertThat(counts.getFailed(), is(0));
     }
 
+    @Test
+    void excludesApiRunsDirectoryFromTheRecursiveWalk(@TempDir File resultsDir) throws IOException {
+        // runtime/logs/test-results/api-runs/<runId>/ accumulates one subdirectory per
+        // API-triggered run (TestRunServices), each already archived under its own runId. A
+        // gradle-triggered run (test/testIntegration) is handed the shared parent directory as
+        // its resultsDir, which sits right next to api-runs/ - without this exclusion, every past
+        // API run's counts would keep getting summed into every future gradle run's manifest too.
+        writeSuiteXml(new File(resultsDir, "SuiteA.xml"), 10, 1, 1, 0);
+        File apiRun = new File(resultsDir, "api-runs/11111111-1111-1111-1111-111111111111");
+        apiRun.mkdirs();
+        writeSuiteXml(new File(apiRun, "contenttests.xml"), 12, 4, 2, 0);
+
+        TestRunManifest.Counts counts = JUnitXmlCounter.count(resultsDir);
+
+        assertThat(counts.getTotal(), is(10));
+        assertThat(counts.getFailed(), is(2));
+    }
+
     @Test
     void returnsAllZeroesWhenDirectoryDoesNotExist() {
         TestRunManifest.Counts counts = JUnitXmlCounter.count(new File("/no/such/dir"));

framework/testtools/src/test/java/org/apache/ofbiz/testtools/report/TestReportArchiverTest.java

@@ -88,6 +88,35 @@ void archivesACombinedResultsDirLikeTestIntegration(@TempDir File tmp) throws IO
         assertThat(new File(runFolder, "results/test-report.html").exists(), is(true));
     }
 
+    @Test
+    void excludesApiRunsDirectoryFromCountsAndFromTheCopiedResults(@TempDir File tmp) throws IOException {
+        // Same scenario JUnitXmlCounterTest exercises at the counter level, checked end-to-end
+        // here: a testIntegration-style resultsDir (the whole runtime/logs/test-results/ tree)
+        // that happens to have api-runs/ sitting in it from earlier API-triggered runs must not
+        // let that unrelated history inflate this run's counts, nor get duplicated into this
+        // run's archived results/ directory.
+        File baseDir = new File(tmp, "runtime/test-reports");
+        File resultsDir = new File(tmp, "runtime/logs/test-results");
+        resultsDir.mkdirs();
+        Files.writeString(new File(resultsDir, "SomeSuite.xml").toPath(),
+                "<testsuite name=\"x\" tests=\"3\" failures=\"0\" errors=\"0\" skipped=\"0\"></testsuite>");
+        File apiRun = new File(resultsDir, "api-runs/11111111-1111-1111-1111-111111111111");
+        apiRun.mkdirs();
+        Files.writeString(new File(apiRun, "contenttests.xml").toPath(),
+                "<testsuite name=\"contenttests\" tests=\"12\" failures=\"4\" errors=\"2\" skipped=\"0\"></testsuite>");
+
+        TestReportArchiver.ArchiveRequest request = new TestReportArchiver.ArchiveRequest(
+                baseDir, tmp, "testIntegration", "testIntegration", "PASSED", resultsDir, null);
+        TestRunManifest manifest = TestReportArchiver.archive(request);
+
+        assertThat(manifest.getCounts().getTotal(), is(3));
+        assertThat(manifest.getCounts().getFailed(), is(0));
+
+        File runFolder = new File(manifest.getResultsLocation());
+        assertThat(new File(runFolder, "results/SomeSuite.xml").exists(), is(true));
+        assertThat(new File(runFolder, "results/api-runs").exists(), is(false));
+    }
+
     @Test
     void runFolderNameFollowsDateTimeSuiteConvention(@TempDir File tmp) throws IOException {
         File baseDir = new File(tmp, "runtime/test-reports");

test-reports.gradle

@@ -251,7 +251,9 @@ def renderStatCards(Map counts, Closure statusHrefFn, List extraCards = [], Stri
     writer.toString()
 }
 
-def buildOverallSummaryTable(List suites, Closure statusHrefFn) {
+// Shared by buildOverallSummaryTable (the HTML report's own summary cards) and createTestReport's
+// console println below, so the two can never disagree about what "the totals" are.
+def computeOverallCounts(List suites) {
     def toInt = { String s -> (s ==~ /\d+/) ? s.toInteger() : 0 }
     def toSeconds = { String s -> (s ==~ /-?[0-9]*\.?[0-9]+/) ? s.toDouble() : 0.0 }
     // List.sum(Closure) returns null (not 0) on an empty list, unlike the no-arg sum() - discovered
@@ -266,10 +268,12 @@ def buildOverallSummaryTable(List suites, Closure statusHrefFn) {
     def successRate = totalTests > 0
             ? String.format('%.2f%%', ((totalTests - totalFailures - totalErrors) / (double) totalTests) * 100)
             : 'N/A'
-    renderStatCards([tests: totalTests, failures: totalFailures, errors: totalErrors,
-                      skipped: totalSkipped, successRate: successRate,
-                      time: String.format('%.3f', totalTime)],
-            statusHrefFn, [], 'overall-summary')
+    [tests: totalTests, failures: totalFailures, errors: totalErrors, skipped: totalSkipped,
+     successRate: successRate, time: String.format('%.3f', totalTime)]
+}
+
+def buildOverallSummaryTable(List suites, Closure statusHrefFn) {
+    renderStatCards(computeOverallCounts(suites), statusHrefFn, [], 'overall-summary')
 }
 
 def buildSummaryTable(List suites, Closure hrefFn) {
@@ -722,6 +726,19 @@ task createTestReport(group: sysadminGroup,
         delete file('./runtime/logs/test-results/modern-report.html')
         reportFile.text = renderModernReport(suites)
         println "Test report written to ${reportFile}"
+
+        // Nothing else in this run's console/log output ever states the totals in one place - a
+        // reader otherwise has to open the HTML report, or hand-sum every suite's own logged
+        // result, to learn whether the run actually passed. Same counts the report's own overall
+        // summary cards show, via the same computeOverallCounts, so the two can never disagree.
+        def counts = computeOverallCounts(suites)
+        def passed = counts.tests - counts.failures - counts.errors
+        def parseErrorCount = suites.count { it.parseError }
+        def parseErrorNote = parseErrorCount > 0
+                ? " (${parseErrorCount} suite(s) could not be parsed and are not included in these counts - see the report)"
+                : ''
+        println "Tests: ${counts.tests}, Passed: ${passed}, Failed: ${counts.failures}, " +
+                "Errors: ${counts.errors}, Skipped: ${counts.skipped}${parseErrorNote}"
     }
 }