Fix junit test runner test report generation

6 files changed, 30 insertions, 27 deletions

diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/AggregateTestRunner.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/AggregateTestRunner.java
index bc4b776568a..6e3393b2761 100644
--- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/AggregateTestRunner.java
+++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/AggregateTestRunner.java
@@ -84,7 +84,6 @@ public class AggregateTestRunner implements TestRunner {
                          .withFailedCount(first.failedCount + second.failedCount)
                          .withIgnoredCount(first.ignoredCount + second.ignoredCount)
                          .withSuccessCount(first.successCount + second.successCount)
-                         .withTotalCount(first.totalCount + second.totalCount)
                          .withFailures(merged(first.failures, second.failures))
                          .withLogs(merged(first.logLines, second.logLines))
                          .build();
diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java
index 929d01ee371..12d50ca725c 100644
--- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java
+++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java
@@ -127,22 +127,22 @@ public class JunitRunner extends AbstractComponent implements TestRunner {
         return logRecords.tailMap(after + 1).values();
     }
 
-    private static TestReport createReportWithFailedInitialization(Exception exception) {
+    static TestReport createReportWithFailedInitialization(Exception exception) {
         TestReport.Failure failure = new TestReport.Failure("init", exception);
-        return new TestReport.Builder()
-                        .withFailures(List.of(failure))
-                        .build();
+        return new TestReport.Builder().withFailures(List.of(failure))
+                                       .withFailedCount(1)
+                                       .build();
     }
 
     private Optional<Bundle> findTestBundle() {
         return Stream.of(bundleContext.getBundles())
-                .filter(this::isTestBundle)
-                .findAny();
+                     .filter(this::isTestBundle)
+                     .findAny();
     }
 
     private boolean isTestBundle(Bundle bundle) {
         var testBundleHeader = bundle.getHeaders().get("X-JDisc-Test-Bundle-Version");
-        return testBundleHeader != null && !testBundleHeader.isBlank();
+        return testBundleHeader != null && ! testBundleHeader.isBlank();
     }
 
     private Optional<TestDescriptor> loadTestDescriptor(Bundle bundle) {
@@ -229,7 +229,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner {
     @Override
     public TestRunner.Status getStatus() {
         if (execution == null) return TestRunner.Status.NOT_STARTED;
-        if (!execution.isDone()) return TestRunner.Status.RUNNING;
+        if ( ! execution.isDone()) return TestRunner.Status.RUNNING;
         try {
             return execution.get().status();
         } catch (InterruptedException|ExecutionException e) {
diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java
index 51e14f6b356..4db5405029b 100644
--- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java
+++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java
@@ -16,7 +16,6 @@ import static com.yahoo.vespa.testrunner.TestRunner.Status.SUCCESS;
  */
 public class TestReport {
 
-    final long totalCount;
     final long successCount;
     final long failedCount;
     final long inconclusiveCount;
@@ -25,8 +24,7 @@ public class TestReport {
     final List<Failure> failures;
     final List<LogRecord> logLines;
 
-    private TestReport(long totalCount, long successCount, long failedCount, long inconclusiveCount, long ignoredCount, long abortedCount, List<Failure> failures, List<LogRecord> logLines) {
-        this.totalCount = totalCount;
+    private TestReport(long successCount, long failedCount, long inconclusiveCount, long ignoredCount, long abortedCount, List<Failure> failures, List<LogRecord> logLines) {
         this.successCount = successCount;
         this.failedCount = failedCount;
         this.inconclusiveCount = inconclusiveCount;
@@ -41,7 +39,7 @@ public class TestReport {
     }
 
     public TestRunner.Status status() {
-        return failedCount > 0 ? FAILURE : inconclusiveCount > 0 ? INCONCLUSIVE : totalCount > 0 ? SUCCESS : NO_TESTS;
+        return failedCount > 0 ? FAILURE : inconclusiveCount > 0 ? INCONCLUSIVE : (successCount + abortedCount + ignoredCount) > 0 ? SUCCESS : NO_TESTS;
     }
 
     public static Builder builder(){
@@ -51,7 +49,6 @@ public class TestReport {
 
     public static class Builder {
 
-        private long totalCount;
         private long successCount;
         private long failedCount;
         private long inconclusiveCount;
@@ -61,12 +58,7 @@ public class TestReport {
         private List<LogRecord> logLines = Collections.emptyList();
 
         public TestReport build() {
-            return new TestReport(totalCount, successCount, failedCount, inconclusiveCount, ignoredCount, abortedCount, failures, logLines);
-        }
-
-        public Builder withTotalCount(long totalCount) {
-            this.totalCount = totalCount;
-            return this;
+            return new TestReport(successCount, failedCount, inconclusiveCount, ignoredCount, abortedCount, failures, logLines);
         }
 
         public Builder withSuccessCount(long successCount) {
diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java
index e7b0fe406c7..3a611e62a75 100644
--- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java
+++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunnerHandler.java
@@ -142,7 +142,6 @@ public class TestRunnerHandler extends ThreadedHttpRequestHandler {
             return slime;
 
         var summary = root.setObject("summary");
-        summary.setLong("total", testReport.totalCount);
         summary.setLong("success", testReport.successCount);
         summary.setLong("failed", testReport.failedCount);
         summary.setLong("ignored", testReport.ignoredCount);
diff --git a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java
index 29d0a1a4b1e..1af012f0fb2 100644
--- a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java
+++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java
@@ -77,7 +77,7 @@ class AggregateTestRunnerTest {
         assertEquals(List.of(), runner.getLog(record1.getSequenceNumber()));
 
         // First wrapped runner completes, second is started.
-        first.status = SUCCESS;
+        first.status = NO_TESTS;
         first.future.complete(null);
         assertNotNull(second.future);
         assertFalse(future.isDone());
@@ -87,8 +87,13 @@ class AggregateTestRunnerTest {
         second.log.add(record2);
         assertEquals(List.of(record1, record2), runner.getLog(-1));
 
-        // No failures means success.
+        // No tests in any runner means no tests.
+        second.status = NO_TESTS;
         second.future.complete(null);
+        assertEquals(NO_TESTS, runner.getStatus());
+
+        // No failures, and at least one success, mean success.
+        second.status = SUCCESS;
         assertEquals(SUCCESS, runner.getStatus());
 
         // An inconclusive test means inconclusive.
@@ -122,7 +127,6 @@ class AggregateTestRunnerTest {
         TestReport report = TestReport.builder()
                                       .withLogs(List.of(record1))
                                       .withFailures(List.of(failure))
-                                      .withTotalCount(15)
                                       .withSuccessCount(8)
                                       .withIgnoredCount(4)
                                       .withFailedCount(2)
@@ -135,13 +139,23 @@ class AggregateTestRunnerTest {
         TestReport merged = runner.getReport();
         assertEquals(List.of(record1, record1), merged.logLines);
         assertEquals(List.of(failure, failure), merged.failures);
-        assertEquals(30, merged.totalCount);
         assertEquals(16, merged.successCount);
         assertEquals(8, merged.ignoredCount);
         assertEquals(4, merged.failedCount);
         assertEquals(2, merged.abortedCount);
     }
 
+    @Test
+    void testReportStatus() {
+        assertEquals(NO_TESTS, TestReport.builder().build().status());
+        assertEquals(SUCCESS, TestReport.builder().withSuccessCount(1).build().status());
+        assertEquals(INCONCLUSIVE, TestReport.builder().withSuccessCount(1).withInconclusiveCount(1).build().status());
+        assertEquals(FAILURE, TestReport.builder().withSuccessCount(1).withFailedCount(1).build().status());
+        assertEquals(SUCCESS, TestReport.builder().withAbortedCount(1).build().status());
+        assertEquals(SUCCESS, TestReport.builder().withIgnoredCount(1).build().status());
+        assertEquals(FAILURE, JunitRunner.createReportWithFailedInitialization(new RuntimeException("hello")).status());
+    }
+
     static class MockTestRunner implements TestRunner {
 
         final List<LogRecord> log = new ArrayList<>();
diff --git a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/TestRunnerHandlerTest.java b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/TestRunnerHandlerTest.java
index 9b08e398564..a3e50e7d5bf 100644
--- a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/TestRunnerHandlerTest.java
+++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/TestRunnerHandlerTest.java
@@ -47,7 +47,6 @@ class TestRunnerHandlerTest {
                 .withFailedCount(2)
                 .withIgnoredCount(3)
                 .withAbortedCount(4)
-                .withTotalCount(10)
                 .withFailures(List.of(new TestReport.Failure("Foo.bar()", exception)))
                 .withLogs(logRecords).build();
 
@@ -60,7 +59,7 @@ class TestRunnerHandlerTest {
         HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/report", GET));
         ByteArrayOutputStream out = new ByteArrayOutputStream();
         response.render(out);
-        JsonTestHelper.assertJsonEquals(out.toString(UTF_8), "{\"summary\":{\"total\":10,\"success\":1,\"failed\":2,\"ignored\":3,\"aborted\":4,\"failures\":[{\"testName\":\"Foo.bar()\",\"testError\":\"org.junit.ComparisonFailure: expected:<foo> but was:<bar>\",\"exception\":\"java.lang.RuntimeException: org.junit.ComparisonFailure: expected:<foo> but was:<bar>\\n\\tat Foo.bar(Foo.java:1123)\\n\"}]},\"output\":[\"Tests started\"]}");
+        JsonTestHelper.assertJsonEquals(out.toString(UTF_8), "{\"summary\":{\"success\":1,\"failed\":2,\"ignored\":3,\"aborted\":4,\"failures\":[{\"testName\":\"Foo.bar()\",\"testError\":\"org.junit.ComparisonFailure: expected:<foo> but was:<bar>\",\"exception\":\"java.lang.RuntimeException: org.junit.ComparisonFailure: expected:<foo> but was:<bar>\\n\\tat Foo.bar(Foo.java:1123)\\n\"}]},\"output\":[\"Tests started\"]}");
     }
 
     @Test