diff --git a/framework/testtools/src/main/java/org/apache/ofbiz/testtools/report/JUnitXmlCounter.java b/framework/testtools/src/main/java/org/apache/ofbiz/testtools/report/JUnitXmlCounter.java index 4bffcf3b25..50540cacf4 100644 --- a/framework/testtools/src/main/java/org/apache/ofbiz/testtools/report/JUnitXmlCounter.java +++ b/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 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); diff --git a/framework/testtools/src/main/java/org/apache/ofbiz/testtools/report/TestReportArchiver.java b/framework/testtools/src/main/java/org/apache/ofbiz/testtools/report/TestReportArchiver.java index d267825fba..523b5dda5c 100644 --- a/framework/testtools/src/main/java/org/apache/ofbiz/testtools/report/TestReportArchiver.java +++ b/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) 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())); } } diff --git a/framework/testtools/src/test/java/org/apache/ofbiz/testtools/report/JUnitXmlCounterTest.java b/framework/testtools/src/test/java/org/apache/ofbiz/testtools/report/JUnitXmlCounterTest.java index 6cb4d97e8e..a8aed40fa5 100644 --- a/framework/testtools/src/test/java/org/apache/ofbiz/testtools/report/JUnitXmlCounterTest.java +++ b/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// 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")); diff --git a/framework/testtools/src/test/java/org/apache/ofbiz/testtools/report/TestReportArchiverTest.java b/framework/testtools/src/test/java/org/apache/ofbiz/testtools/report/TestReportArchiverTest.java index 4ba60ffcf7..4ae3b31b4d 100644 --- a/framework/testtools/src/test/java/org/apache/ofbiz/testtools/report/TestReportArchiverTest.java +++ b/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(), + ""); + File apiRun = new File(resultsDir, "api-runs/11111111-1111-1111-1111-111111111111"); + apiRun.mkdirs(); + Files.writeString(new File(apiRun, "contenttests.xml").toPath(), + ""); + + 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"); diff --git a/test-reports.gradle b/test-reports.gradle index 23aaaeffcb..f649658f68 100644 --- a/test-reports.gradle +++ b/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}" } }