Handle tests using new osgi runner

6 files changed, 165 insertions, 127 deletions

diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitHandler.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitHandler.java
deleted file mode 100644
index cb7d5b8df6b..00000000000
--- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitHandler.java
+++ /dev/null
@@ -1,71 +0,0 @@
-// 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 ai.vespa.hosted.cd.internal.TestRuntimeProvider;
-import com.google.inject.Inject;
-import com.yahoo.container.handler.metrics.JsonResponse;
-import com.yahoo.container.jdisc.HttpRequest;
-import com.yahoo.container.jdisc.HttpResponse;
-import com.yahoo.container.jdisc.LoggingRequestHandler;
-import com.yahoo.container.logging.AccessLog;
-import com.yahoo.restapi.ErrorResponse;
-import com.yahoo.restapi.MessageResponse;
-import org.osgi.framework.Bundle;
-
-import java.io.IOException;
-import java.util.List;
-import java.util.concurrent.Executor;
-import java.util.function.Function;
-
-/**
- * @author mortent
- */
-public class JunitHandler extends LoggingRequestHandler {
-
-    private final JunitRunner junitRunner;
-    private final TestRuntimeProvider testRuntimeProvider;
-
-    @Inject
-    public JunitHandler(Executor executor, AccessLog accessLog, JunitRunner junitRunner, TestRuntimeProvider testRuntimeProvider) {
-        super(executor, accessLog);
-        this.junitRunner = junitRunner;
-        this.testRuntimeProvider = testRuntimeProvider;
-    }
-
-    @Override
-    public HttpResponse handle(HttpRequest httpRequest) {
-        String mode = property("mode", "help", httpRequest, String::valueOf);
-        TestDescriptor.TestCategory category = property("category", TestDescriptor.TestCategory.systemtest, httpRequest, TestDescriptor.TestCategory::valueOf);
-
-        try {
-            testRuntimeProvider.initialize(httpRequest.getData().readAllBytes());
-        } catch (IOException e) {
-            return new ErrorResponse(500, "testruntime-initialization", "Exception reading test config");
-        }
-
-        if ("help".equalsIgnoreCase(mode)) {
-            return new MessageResponse("Accepted modes: \n help \n list \n execute");
-        }
-
-        if (!"list".equalsIgnoreCase(mode) && !"execute".equalsIgnoreCase(mode)) {
-            return new ErrorResponse(400, "client error", "Unknown mode \"" + mode + "\"");
-        }
-
-        Bundle testBundle = junitRunner.findTestBundle("-tests");
-        TestDescriptor testDescriptor = junitRunner.loadTestDescriptor(testBundle);
-        List<Class<?>> testClasses = junitRunner.loadClasses(testBundle, testDescriptor, category);
-
-        String jsonResponse = junitRunner.executeTests(testClasses);
-
-        return new JsonResponse(200, jsonResponse);
-    }
-
-    private static <VAL> VAL property(String name, VAL defaultValue, HttpRequest request, Function<String, VAL> converter) {
-        final String propertyString = request.getProperty(name);
-        if (propertyString != null) {
-            return converter.apply(propertyString);
-        }
-        return defaultValue;
-    }
-}
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 69134f86be0..3fc85365084 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
@@ -2,15 +2,12 @@
 package com.yahoo.vespa.testrunner;
 
 import ai.vespa.hosted.api.TestDescriptor;
+import ai.vespa.hosted.cd.internal.TestRuntimeProvider;
 import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
-import com.yahoo.exception.ExceptionUtils;
 import com.yahoo.io.IOUtils;
 import com.yahoo.jdisc.application.OsgiFramework;
-import com.yahoo.slime.Cursor;
-import com.yahoo.slime.Slime;
-import com.yahoo.slime.SlimeUtils;
-import com.yahoo.yolean.Exceptions;
+import com.yahoo.vespa.testrunner.legacy.LegacyTestRunner;
 import org.junit.jupiter.engine.JupiterTestEngine;
 import org.junit.platform.engine.discovery.DiscoverySelectors;
 import org.junit.platform.launcher.Launcher;
@@ -20,16 +17,19 @@ import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;
 import org.junit.platform.launcher.core.LauncherFactory;
 import org.junit.platform.launcher.listeners.LoggingListener;
 import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
-import org.junit.platform.launcher.listeners.TestExecutionSummary;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 
 import java.io.IOException;
 import java.net.URL;
 import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -41,10 +41,12 @@ public class JunitRunner extends AbstractComponent {
     private static final Logger logger = Logger.getLogger(JunitRunner.class.getName());
 
     private final BundleContext bundleContext;
+    private final TestRuntimeProvider testRuntimeProvider;
+    private Future<TestReport> execution;
 
     @Inject
-    public JunitRunner(OsgiFramework osgiFramework) {
-        // TODO mortent: Find a way to workaround this hack
+    public JunitRunner(OsgiFramework osgiFramework, TestRuntimeProvider testRuntimeProvider) {
+        this.testRuntimeProvider = testRuntimeProvider;
         var tmp = osgiFramework.bundleContext();
         try {
             var field = tmp.getClass().getDeclaredField("wrapped");
@@ -55,27 +57,54 @@ public class JunitRunner extends AbstractComponent {
         }
     }
 
-    public Bundle findTestBundle(String bundleNameSuffix) {
+    public void executeTests(TestDescriptor.TestCategory category, byte[] testConfig) {
+        if (execution != null) {
+            throw new RuntimeException("Test execution already in progress");
+        }
+        testRuntimeProvider.initialize(testConfig);
+        Optional<Bundle> testBundle = findTestBundle();
+        if (testBundle.isEmpty()) {
+            throw new RuntimeException("No test bundle available");
+        }
+
+        Optional<TestDescriptor> testDescriptor = loadTestDescriptor(testBundle.get());
+        if (testDescriptor.isEmpty()) {
+            throw new RuntimeException("Could not find test descriptor");
+        }
+        List<Class<?>> testClasses = loadClasses(testBundle.get(), testDescriptor.get(), category);
+
+        execution =  CompletableFuture.supplyAsync(() -> launchJunit(testClasses));
+    }
+
+    public boolean isSupported() {
+        return findTestBundle().isPresent();
+    }
+
+    private Optional<Bundle> findTestBundle() {
         return Stream.of(bundleContext.getBundles())
-                .filter(bundle -> bundle.getSymbolicName().endsWith(bundleNameSuffix))
-                .findAny()
-                .orElseThrow(() -> new RuntimeException("No bundle on classpath with name ending on " + bundleNameSuffix));
+                .filter(this::isTestBundle)
+                .findAny();
     }
 
-    public TestDescriptor loadTestDescriptor(Bundle bundle) {
+    private boolean isTestBundle(Bundle bundle) {
+        var testBundleHeader = bundle.getHeaders().get("X-JDisc-Test-Bundle-Version");
+        return testBundleHeader != null && !testBundleHeader.isBlank();
+    }
+
+    private Optional<TestDescriptor> loadTestDescriptor(Bundle bundle) {
         URL resource = bundle.getEntry(TestDescriptor.DEFAULT_FILENAME);
         TestDescriptor testDescriptor;
         try {
             var jsonDescriptor = IOUtils.readAll(resource.openStream(), Charset.defaultCharset()).trim();
             testDescriptor = TestDescriptor.fromJsonString(jsonDescriptor);
             logger.info( "Test classes in bundle :" + testDescriptor.toString());
-            return testDescriptor;
+            return Optional.of(testDescriptor);
         } catch (IOException e) {
-            throw new RuntimeException("Could not load " + TestDescriptor.DEFAULT_FILENAME + " [" + e.getMessage() + "]");
+            return Optional.empty();
         }
     }
 
-    public List<Class<?>> loadClasses(Bundle bundle, TestDescriptor testDescriptor, TestDescriptor.TestCategory testCategory) {
+    private List<Class<?>> loadClasses(Bundle bundle, TestDescriptor testDescriptor, TestDescriptor.TestCategory testCategory) {
         List<Class<?>> testClasses = testDescriptor.getConfiguredTests(testCategory).stream()
                 .map(className -> loadClass(bundle, className))
                 .collect(Collectors.toList());
@@ -94,7 +123,7 @@ public class JunitRunner extends AbstractComponent {
         }
     }
 
-    public String executeTests(List<Class<?>> testClasses) {
+    private TestReport launchJunit(List<Class<?>> testClasses) {
         LauncherDiscoveryRequest discoveryRequest = LauncherDiscoveryRequestBuilder.request()
                 .selectors(
                         testClasses.stream().map(DiscoverySelectors::selectClass).collect(Collectors.toList())
@@ -116,36 +145,8 @@ public class JunitRunner extends AbstractComponent {
 
         // Execute request
         launcher.execute(discoveryRequest);
-
         var report = summaryListener.getSummary();
-
-        return createJsonTestReport(report, logLines);
-    }
-
-    private String createJsonTestReport(TestExecutionSummary report, List<String> logLines) {
-        var slime = new Slime();
-        var root = slime.setObject();
-        var summary = root.setObject("summary");
-        summary.setLong("Total tests", report.getTestsFoundCount());
-        summary.setLong("Test success", report.getTestsSucceededCount());
-        summary.setLong("Test failed", report.getTestsFailedCount());
-        summary.setLong("Test ignored", report.getTestsSkippedCount());
-        summary.setLong("Test success", report.getTestsAbortedCount());
-        summary.setLong("Test started", report.getTestsStartedCount());
-        var failures = summary.setArray("failures");
-        report.getFailures().forEach(failure -> serializeFailure(failure, failures.addObject()));
-
-        var output = root.setArray("output");
-        logLines.forEach(output::addString);
-
-        return Exceptions.uncheck(() -> new String(SlimeUtils.toJsonBytes(slime), StandardCharsets.UTF_8));
-    }
-
-    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()));
+        return new TestReport(report, logLines);
     }
 
     private void log(List<String> logs, String message, Throwable t) {
@@ -162,4 +163,33 @@ public class JunitRunner extends AbstractComponent {
     public void deconstruct() {
         super.deconstruct();
     }
+
+    public LegacyTestRunner.Status getStatus() {
+        if (execution == null) return LegacyTestRunner.Status.NOT_STARTED;
+        if (!execution.isDone()) return LegacyTestRunner.Status.RUNNING;
+        try {
+            TestReport report = execution.get();
+            if (report.isSuccess()) {
+                return LegacyTestRunner.Status.SUCCESS;
+            } else {
+                return LegacyTestRunner.Status.FAILURE;
+            }
+        } catch (InterruptedException|ExecutionException e) {
+            logger.log(Level.WARNING, "Error while getting test report", e);
+            return LegacyTestRunner.Status.ERROR;
+        }
+    }
+
+    public String getReportAsJson() {
+        if (execution.isDone()) {
+            try {
+                return execution.get().toJson();
+            } catch (Exception e) {
+                logger.log(Level.WARNING, "Error getting test report", e);
+                return "";
+            }
+        } else {
+            return "";
+        }
+    }
 }
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
new file mode 100644
index 00000000000..2e45ba96486
--- /dev/null
+++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java
@@ -0,0 +1,55 @@
+// 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 com.yahoo.exception.ExceptionUtils;
+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.List;
+
+/**
+ * @author mortent
+ */
+public class TestReport {
+    private final TestExecutionSummary junitReport;
+    private final List<String> logLines;
+
+    public TestReport(TestExecutionSummary junitReport, List<String> logLines) {
+        this.junitReport = junitReport;
+        this.logLines = List.copyOf(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()));
+    }
+
+    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()));
+
+        var output = root.setArray("output");
+        logLines.forEach(output::addString);
+
+        return Exceptions.uncheck(() -> new String(SlimeUtils.toJsonBytes(slime), StandardCharsets.UTF_8));
+    }
+
+    public boolean isSuccess() {
+        return (junitReport.getTestsFailedCount() + junitReport.getTestsAbortedCount()) == 0;
+    }
+}
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 758ce110766..2a827659695 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
@@ -19,7 +19,9 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import java.util.concurrent.Executor;
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
@@ -67,12 +69,27 @@ public class TestRunnerHandler extends LoggingRequestHandler {
     private HttpResponse handleGET(HttpRequest request) {
         String path = request.getUri().getPath();
         if (path.equals("/tester/v1/log")) {
-            return new SlimeJsonResponse(logToSlime(testRunner.getLog(request.hasProperty("after")
-                                                                               ? Long.parseLong(request.getProperty("after"))
-                                                                               : -1)));
+            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));
+                }
+                return new SlimeJsonResponse(logToSlime(logRecords));
+            } else {
+                return new SlimeJsonResponse(logToSlime(testRunner.getLog(request.hasProperty("after")
+                        ? Long.parseLong(request.getProperty("after"))
+                        : -1)));
+            }
         } else if (path.equals("/tester/v1/status")) {
-            log.info("Responding with status " + testRunner.getStatus());
-            return new Response(testRunner.getStatus().name());
+            if (useOsgiMode) {
+                log.info("Responding with status " + junitRunner.getStatus());
+                return new Response(junitRunner.getStatus().name());
+            } else {
+                log.info("Responding with status " + testRunner.getStatus());
+                return new Response(testRunner.getStatus().name());
+            }
         }
         return new Response(Status.NOT_FOUND, "Not found: " + request.getUri().getPath());
     }
@@ -83,9 +100,15 @@ public class TestRunnerHandler extends LoggingRequestHandler {
             String type = lastElement(path);
             TestProfile testProfile = TestProfile.valueOf(type.toUpperCase() + "_TEST");
             byte[] config = request.getData().readAllBytes();
-            testRunner.test(testProfile, config);
-            log.info("Started tests of type " + type + " and status is " + testRunner.getStatus());
-            return new Response("Successfully started " + type + " tests");
+            if (useOsgiMode) {
+                junitRunner.executeTests(testProfile.testCategory(), config);
+                log.info("Started tests of type " + type + " and status is " + junitRunner.getStatus());
+                return new Response("Successfully started " + type + " tests");
+            } else {
+                testRunner.test(testProfile, config);
+                log.info("Started tests of type " + type + " and status is " + testRunner.getStatus());
+                return new Response("Successfully started " + type + " tests");
+            }
         }
         return new Response(Status.NOT_FOUND, "Not found: " + request.getUri().getPath());
     }
diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/LegacyTestRunner.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/LegacyTestRunner.java
index d3777152590..9f1a68218f0 100644
--- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/LegacyTestRunner.java
+++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/LegacyTestRunner.java
@@ -15,6 +15,7 @@ public interface LegacyTestRunner {
 
     void test(TestProfile testProfile, byte[] config);
 
+    // TODO (mortent) : This seems to be duplicated in TesterCloud.Status and expects to have the same values
     enum Status {
         NOT_STARTED, RUNNING, FAILURE, ERROR, SUCCESS
     }
diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/TestProfile.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/TestProfile.java
index 60f4c15c40d..f3173d6758c 100644
--- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/TestProfile.java
+++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/TestProfile.java
@@ -31,7 +31,7 @@ public enum TestProfile {
         return failIfNoTests;
     }
 
-    TestDescriptor.TestCategory testCategory() {
+    public TestDescriptor.TestCategory testCategory() {
         return testCategory;
     }