Add test report api to test runner

6 files changed, 330 insertions, 45 deletions

diff --git a/vespa-osgi-testrunner/pom.xml b/vespa-osgi-testrunner/pom.xml
index db0dba89b8a..41b938a6e0e 100644
--- a/vespa-osgi-testrunner/pom.xml
+++ b/vespa-osgi-testrunner/pom.xml
@@ -57,6 +57,17 @@
             <version>${project.version}</version>
             <scope>compile</scope>
         </dependency>
+        <dependency>
+            <groupId>com.yahoo.vespa</groupId>
+            <artifactId>testutil</artifactId>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <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 7c86347a5b9..94ddd0a7f87 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
@@ -31,6 +31,7 @@ import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.logging.Level;
+import java.util.logging.LogRecord;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -38,7 +39,7 @@ import java.util.stream.Stream;
 /**
  * @author mortent
  */
-public class JunitRunner extends AbstractComponent {
+public class JunitRunner extends AbstractComponent implements TestRunner {
     private static final Logger logger = Logger.getLogger(JunitRunner.class.getName());
 
     private final BundleContext bundleContext;
@@ -77,6 +78,7 @@ public class JunitRunner extends AbstractComponent {
         System.setProperty("vespa.test.credentials.root", credentialsRoot);
     }
 
+    @Override
     public void executeTests(TestDescriptor.TestCategory category, byte[] testConfig) {
         if (execution != null && !execution.isDone()) {
             throw new IllegalStateException("Test execution already in progress");
@@ -96,6 +98,7 @@ public class JunitRunner extends AbstractComponent {
         execution =  CompletableFuture.supplyAsync(() -> launchJunit(testClasses));
     }
 
+    @Override
     public boolean isSupported() {
         return findTestBundle().isPresent();
     }
@@ -157,7 +160,7 @@ public class JunitRunner extends AbstractComponent {
         Launcher launcher = LauncherFactory.create(launcherConfig);
 
         // Create log listener:
-        var logLines = new ArrayList<String>();
+        var logLines = new ArrayList<LogRecord>();
         var logListener = LoggingListener.forBiConsumer((t, m) -> log(logLines, m.get(), t));
         // Create a summary listener:
         var summaryListener = new SummaryGeneratingListener();
@@ -166,17 +169,23 @@ public class JunitRunner extends AbstractComponent {
         // Execute request
         launcher.execute(discoveryRequest);
         var report = summaryListener.getSummary();
-        return new TestReport(report, logLines);
+        var failures = report.getFailures().stream()
+                .map(failure -> new TestReport.Failure(failure.getTestIdentifier().getUniqueId(), failure.getException()))
+                .collect(Collectors.toList());
+        return TestReport.builder()
+                .withSuccessCount(report.getTestsSucceededCount())
+                .withAbortedCount(report.getTestsAbortedCount())
+                .withIgnoredCount(report.getTestsSkippedCount())
+                .withFailedCount(report.getTestsFailedCount())
+                .withFailures(failures)
+                .withLogs(logLines)
+                .build();
     }
 
-    private void log(List<String> logs, String message, Throwable t) {
-        logs.add(message);
-        if(t != null) {
-            logs.add(t.getMessage());
-            List.of(t.getStackTrace()).stream()
-                    .map(StackTraceElement::toString)
-                    .forEach(logs::add);
-        }
+    private void log(List<LogRecord> logs, String message, Throwable t) {
+        LogRecord logRecord = new LogRecord(Level.INFO, message);
+        Optional.ofNullable(t).ifPresent(logRecord::setThrown);
+        logs.add(logRecord);
     }
 
     @Override
@@ -184,6 +193,7 @@ public class JunitRunner extends AbstractComponent {
         super.deconstruct();
     }
 
+    @Override
     public LegacyTestRunner.Status getStatus() {
         if (execution == null) return LegacyTestRunner.Status.NOT_STARTED;
         if (!execution.isDone()) return LegacyTestRunner.Status.RUNNING;
@@ -200,16 +210,24 @@ public class JunitRunner extends AbstractComponent {
         }
     }
 
-    public String getReportAsJson() {
+    @Override
+    public TestReport getReport() {
         if (execution.isDone()) {
             try {
-                return execution.get().toJson();
+                return execution.get();
             } catch (Exception e) {
                 logger.log(Level.WARNING, "Error getting test report", e);
-                return "";
+                return null;
             }
         } else {
-            return "";
+            return null;
         }
     }
+
+    @Override
+    public String getReportAsJson() {
+        return Optional.ofNullable(getReport())
+                .map(TestReport::toJson)
+                .orElse("");
+    }
 }
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 2e45ba96486..807c9907c06 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
@@ -6,50 +6,131 @@ import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Slime;
 import com.yahoo.slime.SlimeUtils;
 import com.yahoo.yolean.Exceptions;
-import org.junit.platform.launcher.listeners.TestExecutionSummary;
 
 import java.nio.charset.StandardCharsets;
+import java.util.Collections;
 import java.util.List;
+import java.util.logging.LogRecord;
 
 /**
  * @author mortent
  */
 public class TestReport {
-    private final TestExecutionSummary junitReport;
-    private final List<String> logLines;
+    private final long totalCount;
+    private final long successCount;
+    private final long failedCount;
+    private final long ignoredCount;
+    private final long abortedCount;
+    private final List<Failure> failures;
+    private final List<LogRecord> logLines;
 
-    public TestReport(TestExecutionSummary junitReport, List<String> logLines) {
-        this.junitReport = junitReport;
-        this.logLines = List.copyOf(logLines);
+    public TestReport(long totalCount, long successCount, long failedCount, long ignoredCount, long abortedCount, List<Failure> failures, List<LogRecord> logLines) {
+        this.totalCount = totalCount;
+        this.successCount = successCount;
+        this.failedCount = failedCount;
+        this.ignoredCount = ignoredCount;
+        this.abortedCount = abortedCount;
+        this.failures = failures;
+        this.logLines = logLines;
     }
 
-    private void serializeFailure(TestExecutionSummary.Failure failure, Cursor slime) {
-        var testIdentifier = failure.getTestIdentifier();
-        slime.setString("testName", testIdentifier.getUniqueId());
-        slime.setString("testError",failure.getException().getMessage());
-        slime.setString("exception", ExceptionUtils.getStackTraceAsString(failure.getException()));
+    private void serializeFailure(Failure failure, Cursor slime) {
+        var testIdentifier = failure.testId();
+        slime.setString("testName", failure.testId());
+        slime.setString("testError",failure.exception().getMessage());
+        slime.setString("exception", ExceptionUtils.getStackTraceAsString(failure.exception()));
     }
 
     public String toJson() {
         var slime = new Slime();
         var root = slime.setObject();
         var summary = root.setObject("summary");
-        summary.setLong("Total tests", junitReport.getTestsFoundCount());
-        summary.setLong("Test success", junitReport.getTestsSucceededCount());
-        summary.setLong("Test failed", junitReport.getTestsFailedCount());
-        summary.setLong("Test ignored", junitReport.getTestsSkippedCount());
-        summary.setLong("Test aborted", junitReport.getTestsAbortedCount());
-        summary.setLong("Test started", junitReport.getTestsStartedCount());
-        var failures = summary.setArray("failures");
-        junitReport.getFailures().forEach(failure -> serializeFailure(failure, failures.addObject()));
+        summary.setLong("total", totalCount);
+        summary.setLong("success", successCount);
+        summary.setLong("failed", failedCount);
+        summary.setLong("ignored", ignoredCount);
+        summary.setLong("aborted", abortedCount);
+        var failureRoot = summary.setArray("failures");
+        this.failures.forEach(failure -> serializeFailure(failure, failureRoot.addObject()));
 
         var output = root.setArray("output");
-        logLines.forEach(output::addString);
+        logLines.forEach(lr -> output.addString(lr.getMessage()));
 
         return Exceptions.uncheck(() -> new String(SlimeUtils.toJsonBytes(slime), StandardCharsets.UTF_8));
     }
 
+    public List<LogRecord> logLines() {
+        return logLines;
+    }
+
     public boolean isSuccess() {
-        return (junitReport.getTestsFailedCount() + junitReport.getTestsAbortedCount()) == 0;
+        return (failedCount + abortedCount) == 0;
+    }
+
+    public static Builder builder(){
+        return new Builder();
+    }
+
+    public static class Builder {
+        private long totalCount;
+        private long successCount;
+        private long failedCount;
+        private long ignoredCount;
+        private long abortedCount;
+        private List<Failure> failures = Collections.emptyList();
+        private List<LogRecord> logLines = Collections.emptyList();
+
+        public TestReport build() {
+            return new TestReport(totalCount, successCount, failedCount, ignoredCount, abortedCount, failures, logLines);
+        }
+
+        public Builder withTotalCount(long totalCount) {
+            this.totalCount = totalCount;
+            return this;
+        }
+        public Builder withSuccessCount(long successCount) {
+            this.successCount = successCount;
+            return this;
+        }
+        public Builder withFailedCount(long failedCount) {
+            this.failedCount = failedCount;
+            return this;
+        }
+        public Builder withIgnoredCount(long ignoredCount) {
+            this.ignoredCount = ignoredCount;
+            return this;
+        }
+        public Builder withAbortedCount(long abortedCount) {
+            this.abortedCount = abortedCount;
+            return this;
+        }
+
+        public Builder withFailures(List<Failure> failures) {
+            this.failures = List.copyOf(failures);
+            return this;
+        }
+
+        public Builder withLogs(List<LogRecord> logRecords) {
+            this.logLines = logRecords;
+            return this;
+        }
+    }
+
+    public static class Failure {
+        private final String testId;
+        private final Throwable exception;
+
+        public Failure(String testId, Throwable exception) {
+            this.testId = testId;
+            this.exception = exception;
+        }
+
+        public String testId() {
+            return testId;
+        }
+
+        public Throwable exception() {
+            return exception;
+        }
     }
 }
diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunner.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunner.java
new file mode 100644
index 00000000000..210d05691de
--- /dev/null
+++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestRunner.java
@@ -0,0 +1,20 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.testrunner;
+
+import ai.vespa.hosted.api.TestDescriptor;
+import com.yahoo.vespa.testrunner.legacy.LegacyTestRunner;
+
+/**
+ * @author mortent
+ */
+public interface TestRunner {
+    void executeTests(TestDescriptor.TestCategory category, byte[] testConfig);
+
+    boolean isSupported();
+
+    LegacyTestRunner.Status getStatus();
+
+    TestReport getReport();
+
+    String getReportAsJson();
+}
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 cb337a0c176..76e72865d26 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
@@ -5,6 +5,7 @@ import ai.vespa.hosted.api.TestDescriptor;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.inject.Inject;
+import com.yahoo.container.jdisc.EmptyResponse;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
@@ -12,6 +13,7 @@ import com.yahoo.container.logging.AccessLog;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.JsonFormat;
 import com.yahoo.slime.Slime;
+import com.yahoo.slime.SlimeUtils;
 import com.yahoo.vespa.testrunner.legacy.LegacyTestRunner;
 import com.yahoo.vespa.testrunner.legacy.TestProfile;
 import com.yahoo.yolean.Exceptions;
@@ -20,12 +22,14 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
-import java.util.ArrayList;
+import java.time.Instant;
 import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.Executor;
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
+import java.util.stream.Collectors;
 
 import static com.yahoo.jdisc.Response.Status;
 
@@ -38,12 +42,12 @@ public class TestRunnerHandler extends LoggingRequestHandler {
 
     private static final String CONTENT_TYPE_APPLICATION_JSON = "application/json";
 
-    private final JunitRunner junitRunner;
+    private final TestRunner junitRunner;
     private final LegacyTestRunner testRunner;
     private final boolean useOsgiMode;
 
     @Inject
-    public TestRunnerHandler(Executor executor, AccessLog accessLog, JunitRunner junitRunner, LegacyTestRunner testRunner) {
+    public TestRunnerHandler(Executor executor, AccessLog accessLog, TestRunner junitRunner, LegacyTestRunner testRunner) {
         super(executor, accessLog);
         this.junitRunner = junitRunner;
         this.testRunner = testRunner;
@@ -71,12 +75,14 @@ public class TestRunnerHandler extends LoggingRequestHandler {
         String path = request.getUri().getPath();
         if (path.equals("/tester/v1/log")) {
             if (useOsgiMode) {
-                // TODO (mortent): Handle case where log is returned multiple times
-                String report = junitRunner.getReportAsJson();
-                List<LogRecord> logRecords = new ArrayList<>();
-                if (!report.isBlank()) {
-                    logRecords.add(new LogRecord(Level.INFO, report));
-                }
+                TestReport report = junitRunner.getReport();
+                Instant fetchRecordsAfter = Optional.ofNullable(request.getProperty("after"))
+                        .map(Long::parseLong)
+                        .map(Instant::ofEpochSecond)
+                        .orElse(Instant.EPOCH);
+                List<LogRecord> logRecords = report.logLines().stream()
+                        .filter(record -> record.getInstant().isAfter(fetchRecordsAfter))
+                        .collect(Collectors.toList());
                 return new SlimeJsonResponse(logToSlime(logRecords));
             } else {
                 return new SlimeJsonResponse(logToSlime(testRunner.getLog(request.hasProperty("after")
@@ -91,6 +97,13 @@ public class TestRunnerHandler extends LoggingRequestHandler {
                 log.info("Responding with status " + testRunner.getStatus());
                 return new Response(testRunner.getStatus().name());
             }
+        } else if (path.equals("/tester/v1/report")) {
+            if (useOsgiMode) {
+                String report = junitRunner.getReportAsJson();
+                return new SlimeJsonResponse(SlimeUtils.jsonToSlime(report));
+            } else {
+                return new EmptyResponse(200);
+            }
         }
         return new Response(Status.NOT_FOUND, "Not found: " + request.getUri().getPath());
     }
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
new file mode 100644
index 00000000000..580404d3744
--- /dev/null
+++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/TestRunnerHandlerTest.java
@@ -0,0 +1,142 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.testrunner;
+
+import ai.vespa.hosted.api.TestDescriptor;
+import com.yahoo.container.jdisc.HttpRequest;
+import com.yahoo.container.jdisc.HttpResponse;
+import com.yahoo.container.logging.AccessLog;
+import com.yahoo.test.json.JsonTestHelper;
+import com.yahoo.vespa.testrunner.legacy.LegacyTestRunner;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.time.Instant;
+import java.util.List;
+import java.util.concurrent.Executors;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+
+import static com.yahoo.jdisc.http.HttpRequest.Method.GET;
+import static org.junit.Assert.assertEquals;
+
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author mortent
+ */
+public class TestRunnerHandlerTest {
+
+    private static final Instant testInstant = Instant.ofEpochMilli(1598432151660L);
+    private static TestRunnerHandler testRunnerHandler;
+
+    @BeforeAll
+    public static void setup() {
+        List<LogRecord> logRecords = List.of(logRecord("Tests started"));
+        Throwable exception = new RuntimeException("org.junit.ComparisonFailure: expected:<foo> but was:<bar>");
+        exception.setStackTrace(new StackTraceElement[]{new StackTraceElement("Foo", "bar", "Foo.java", 1123)});
+        TestReport testReport = TestReport.builder()
+                .withSuccessCount(1)
+                .withFailedCount(2)
+                .withIgnoredCount(3)
+                .withAbortedCount(4)
+                .withTotalCount(10)
+                .withFailures(List.of(new TestReport.Failure("Foo.bar()", exception)))
+                .withLogs(logRecords).build();
+
+        testRunnerHandler = new TestRunnerHandler(
+                Executors.newSingleThreadExecutor(),
+                AccessLog.voidAccessLog(),
+                new MockJunitRunner(LegacyTestRunner.Status.SUCCESS, testReport),
+                null);
+    }
+
+    @Test
+    public void createsCorrectTestReport() throws IOException {
+        HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/report", GET));
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        response.render(out);
+        JsonTestHelper.assertJsonEquals(new String(out.toByteArray()), "{\"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\"]}");
+
+    }
+
+    @Test
+    public void returnsCorrectLog() throws IOException {
+        HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/log", GET));
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        response.render(out);
+        JsonTestHelper.assertJsonEquals(new String(out.toByteArray()), "{\"logRecords\":[{\"id\":0,\"at\":1598432151660,\"type\":\"info\",\"message\":\"Tests started\"}]}");
+
+        // Should not get old log
+        response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/log?after="+testInstant.plusSeconds(1).getEpochSecond(), GET));
+        out = new ByteArrayOutputStream();
+        response.render(out);
+        assertEquals("{\"logRecords\":[]}", new String(out.toByteArray()));
+
+    }
+
+    @Test
+    public void usesLegacyTestRunnerWhenNotSupported() throws IOException {
+        TestRunner testRunner = mock(TestRunner.class);
+        when(testRunner.isSupported()).thenReturn(false);
+        LegacyTestRunner legacyTestRunner = mock(LegacyTestRunner.class);
+        when(legacyTestRunner.getLog(anyLong())).thenReturn(List.of(logRecord("Legacy log message")));
+
+        testRunnerHandler = new TestRunnerHandler(
+                Executors.newSingleThreadExecutor(),
+                AccessLog.voidAccessLog(),
+                testRunner, legacyTestRunner);
+
+        HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/log", GET));
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        response.render(out);
+        JsonTestHelper.assertJsonEquals(new String(out.toByteArray()), "{\"logRecords\":[{\"id\":0,\"at\":1598432151660,\"type\":\"info\",\"message\":\"Legacy log message\"}]}");
+    }
+
+    /* Creates a LogRecord that has a known instant and sequence number to get predictable serialization format */
+    private static LogRecord logRecord(String logMessage) {
+        LogRecord logRecord = new LogRecord(Level.INFO, logMessage);
+        logRecord.setInstant(testInstant);
+        logRecord.setSequenceNumber(0);
+        return logRecord;
+    }
+
+    private static class MockJunitRunner implements TestRunner {
+
+        private final LegacyTestRunner.Status status;
+        private final TestReport testReport;
+
+        public MockJunitRunner(LegacyTestRunner.Status status, TestReport testReport) {
+
+            this.status = status;
+            this.testReport = testReport;
+        }
+
+        @Override
+        public void executeTests(TestDescriptor.TestCategory category, byte[] testConfig) {
+        }
+
+        @Override
+        public boolean isSupported() {
+            return true;
+        }
+
+        @Override
+        public LegacyTestRunner.Status getStatus() {
+            return status;
+        }
+
+        @Override
+        public TestReport getReport() {
+            return testReport;
+        }
+
+        @Override
+        public String getReportAsJson() {
+            return getReport().toJson();
+        }
+    }
+}