Add NO_TESTS status to simplify detecting that no tests are available

8 files changed, 66 insertions, 117 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 5637d25df71..bc4b776568a 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
@@ -7,24 +7,13 @@ import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.logging.Level;
 import java.util.logging.LogRecord;
 
-import static java.util.stream.Collectors.toUnmodifiableList;
-
 /**
  * @author jonmv
  */
 public class AggregateTestRunner implements TestRunner {
 
-    static final TestRunner noRunner = new TestRunner() {
-        final LogRecord record = new LogRecord(Level.WARNING, "No tests were found");
-        @Override public Collection<LogRecord> getLog(long after) { return List.of(record); }
-        @Override public Status getStatus() { return Status.FAILURE; }
-        @Override public CompletableFuture<?> test(Suite suite, byte[] config) { return CompletableFuture.completedFuture(null); }
-        @Override public boolean isSupported() { return true; }
-    };
-
     private final List<TestRunner> wrapped;
     private final AtomicInteger current = new AtomicInteger(-1);
 
@@ -33,8 +22,7 @@ public class AggregateTestRunner implements TestRunner {
     }
 
     public static TestRunner of(Collection<TestRunner> testRunners) {
-        List<TestRunner> supported = testRunners.stream().filter(TestRunner::isSupported).collect(toUnmodifiableList());
-        return supported.isEmpty() ? noRunner : new AggregateTestRunner(supported);
+        return new AggregateTestRunner(List.copyOf(testRunners));
     }
 
     @Override
@@ -51,17 +39,13 @@ public class AggregateTestRunner implements TestRunner {
         if (current.get() == -1)
             return Status.NOT_STARTED;
 
-        boolean failed = false;
-        boolean inconclusive = false;
+        Status status = Status.NO_TESTS;
         for (int i = 0; i <= current.get(); i++) {
             if (i == wrapped.size())
-                return failed ? Status.FAILURE : inconclusive ? Status.INCONCLUSIVE : Status.SUCCESS;
+                return status;
 
-            switch (wrapped.get(i).getStatus()) {
-                case ERROR: return Status.ERROR;
-                case INCONCLUSIVE: inconclusive = true; break;
-                case FAILURE: failed = true;
-            }
+            Status next = wrapped.get(i).getStatus();
+            status = status.ordinal() < next.ordinal() ? status : next;
         }
         return Status.RUNNING;
     }
@@ -89,11 +73,6 @@ public class AggregateTestRunner implements TestRunner {
     }
 
     @Override
-    public boolean isSupported() {
-        return wrapped.stream().anyMatch(TestRunner::isSupported);
-    }
-
-    @Override
     public TestReport getReport() {
         return wrapped.stream().map(TestRunner::getReport).filter(Objects::nonNull)
                       .reduce(AggregateTestRunner::merge).orElse(null);
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 58f201cd599..929d01ee371 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
@@ -103,12 +103,13 @@ public class JunitRunner extends AbstractComponent implements TestRunner {
         }
         try {
             logRecords.clear();
-            testRuntimeProvider.initialize(testConfig);
             Optional<Bundle> testBundle = findTestBundle();
             if (testBundle.isEmpty()) {
-                throw new RuntimeException("No test bundle available");
+                execution = CompletableFuture.completedFuture(TestReport.builder().build());
+                return execution;
             }
 
+            testRuntimeProvider.initialize(testConfig);
             Optional<TestDescriptor> testDescriptor = loadTestDescriptor(testBundle.get());
             if (testDescriptor.isEmpty()) {
                 throw new RuntimeException("Could not find test descriptor");
@@ -133,11 +134,6 @@ public class JunitRunner extends AbstractComponent implements TestRunner {
                         .build();
     }
 
-    @Override
-    public boolean isSupported() {
-        return findTestBundle().isPresent();
-    }
-
     private Optional<Bundle> findTestBundle() {
         return Stream.of(bundleContext.getBundles())
                 .filter(this::isTestBundle)
@@ -169,7 +165,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner {
 
         StringBuffer buffer = new StringBuffer();
         testClasses.forEach(cl -> buffer.append("\t").append(cl.toString()).append(" / ").append(cl.getClassLoader().toString()).append("\n"));
-        logger.info("Loaded testClasses: \n" + buffer.toString());
+        logger.info("Loaded testClasses: \n" + buffer);
         return testClasses;
     }
 
@@ -183,9 +179,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner {
 
     private TestReport launchJunit(List<Class<?>> testClasses, boolean isProductionTest) {
         LauncherDiscoveryRequest discoveryRequest = LauncherDiscoveryRequestBuilder.request()
-                .selectors(
-                        testClasses.stream().map(DiscoverySelectors::selectClass).collect(Collectors.toList())
-                )
+                .selectors(testClasses.stream().map(DiscoverySelectors::selectClass).collect(Collectors.toList()))
                 .build();
 
         var launcherConfig = LauncherConfig.builder()
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 8cced4f8cd6..51e14f6b356 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
@@ -8,6 +8,7 @@ import java.util.logging.LogRecord;
 
 import static com.yahoo.vespa.testrunner.TestRunner.Status.FAILURE;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.INCONCLUSIVE;
+import static com.yahoo.vespa.testrunner.TestRunner.Status.NO_TESTS;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.SUCCESS;
 
 /**
@@ -40,7 +41,7 @@ public class TestReport {
     }
 
     public TestRunner.Status status() {
-        return failedCount > 0 ? FAILURE : inconclusiveCount > 0 ? INCONCLUSIVE : SUCCESS;
+        return failedCount > 0 ? FAILURE : inconclusiveCount > 0 ? INCONCLUSIVE : totalCount > 0 ? SUCCESS : NO_TESTS;
     }
 
     public static Builder builder(){
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
index 20ff19266cf..c38226f3c27 100644
--- 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
@@ -17,13 +17,33 @@ public interface TestRunner {
 
     CompletableFuture<?> test(Suite suite, byte[] config);
 
-    boolean isSupported();
-
     default TestReport getReport() { return null; }
 
+    /** Test run status, ordered from most to least specific; the most specific result is chosen when combining multiple. */
     enum Status {
-        NOT_STARTED, RUNNING, FAILURE, INCONCLUSIVE, ERROR, SUCCESS
-    }
+
+        /** Tests are currently running. */
+        RUNNING,
+
+        /** Framework exception; never got to run the tests, or failed parsing their output. */
+        ERROR,
+
+        /** Test code failed. */
+        FAILURE,
+
+        /** Tests should be re-run at a later time. */
+        INCONCLUSIVE,
+
+        /** All tests passed. */
+        SUCCESS,
+
+        /** No tests found. */
+        NO_TESTS,
+
+        /** Tests have not yet started. */
+        NOT_STARTED
+
+        }
 
     enum Suite {
         SYSTEM_TEST, STAGING_SETUP_TEST, STAGING_TEST, PRODUCTION_TEST
diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/VespaCliTestRunner.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/VespaCliTestRunner.java
index 354d2476afb..c01286a8064 100644
--- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/VespaCliTestRunner.java
+++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/VespaCliTestRunner.java
@@ -27,6 +27,7 @@ import java.util.stream.Stream;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.ERROR;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.FAILURE;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.INCONCLUSIVE;
+import static com.yahoo.vespa.testrunner.TestRunner.Status.NO_TESTS;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.RUNNING;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.SUCCESS;
 import static com.yahoo.yolean.Exceptions.uncheck;
@@ -74,28 +75,23 @@ public class VespaCliTestRunner implements TestRunner {
         return CompletableFuture.runAsync(() -> runTests(suite, config));
     }
 
-    @Override
-    public boolean isSupported() {
-        return Stream.of(Suite.SYSTEM_TEST, Suite.STAGING_SETUP_TEST, Suite.STAGING_TEST)
-                     .anyMatch(suite -> getChildDirectory(testsPath, toSuiteDirectoryName(suite)).isPresent());
-    }
-
     void runTests(Suite suite, byte[] config) {
         Process process = null;
         try {
             TestConfig testConfig = TestConfig.fromJson(config);
-            process = testRunProcessBuilder(suite, testConfig).start();
+            ProcessBuilder builder = testRunProcessBuilder(suite, testConfig);
+            if (builder == null) {
+                status.set(NO_TESTS);
+                return;
+            }
 
+            process = builder.start();
             HtmlLogger htmlLogger = new HtmlLogger();
             BufferedReader in = new BufferedReader(new InputStreamReader(process.getInputStream()));
             in.lines().forEach(line -> log(htmlLogger.toLog(line)));
             int exitCode = process.waitFor();
             status.set(exitCode == 0 ? SUCCESS : FAILURE);
         }
-        catch (NoTestsException e) {
-            log(Level.WARNING, "Did not find expected basic HTTP tests", e);
-            status.set(FAILURE);
-        }
         catch (Exception e) {
             if (process != null)
                 process.destroyForcibly();
@@ -114,10 +110,11 @@ public class VespaCliTestRunner implements TestRunner {
     }
 
     ProcessBuilder testRunProcessBuilder(Suite suite, TestConfig config) throws IOException {
-        Path suitePath = getChildDirectory(testsPath, toSuiteDirectoryName(suite))
-                .orElseThrow(() -> new NoTestsException("No tests found, for suite '" + suite + "'"));
+        Optional<Path> suitePath = getChildDirectory(testsPath, toSuiteDirectoryName(suite));
+        if (suitePath.isEmpty())
+            return null;
 
-        ProcessBuilder builder = new ProcessBuilder("vespa", "test", suitePath.toAbsolutePath().toString(),
+        ProcessBuilder builder = new ProcessBuilder("vespa", "test", suitePath.get().toAbsolutePath().toString(),
                                                     "--application", config.application().toFullString(),
                                                     "--zone", config.zone().value(),
                                                     "--target", "cloud");
@@ -172,10 +169,4 @@ public class VespaCliTestRunner implements TestRunner {
         return new String(SlimeUtils.toJsonBytes(root), UTF_8);
     }
 
-    static class NoTestsException extends RuntimeException {
-
-        private NoTestsException(String message) { super(message); }
-
-    }
-
 }
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 a7ddbbf3fcc..29d0a1a4b1e 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
@@ -14,6 +14,7 @@ import static com.yahoo.vespa.testrunner.TestRunner.Status.ERROR;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.FAILURE;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.INCONCLUSIVE;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.NOT_STARTED;
+import static com.yahoo.vespa.testrunner.TestRunner.Status.NO_TESTS;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.RUNNING;
 import static com.yahoo.vespa.testrunner.TestRunner.Status.SUCCESS;
 import static java.util.stream.Collectors.toList;
@@ -31,30 +32,34 @@ class AggregateTestRunnerTest {
 
     @Test
     void onlySupportedRunnersAreUsed() {
-        MockTestRunner unsupported = new MockTestRunner(false);
-        MockTestRunner suppported = new MockTestRunner(true);
-        TestRunner runner = AggregateTestRunner.of(List.of(unsupported, suppported));
+        MockTestRunner unsupported = new MockTestRunner();
+        unsupported.status = NO_TESTS;
+        MockTestRunner supported = new MockTestRunner();
+        supported.status = SUCCESS;
+        TestRunner runner = AggregateTestRunner.of(List.of(unsupported, supported));
         CompletableFuture<?> future = runner.test(null, null);
         assertFalse(future.isDone());
-        assertNull(unsupported.future);
-        assertNotNull(suppported.future);
-        suppported.future.complete(null);
+        assertNotNull(unsupported.future);
+        assertNull(supported.future);
+        unsupported.future.complete(null);
+        assertNotNull(supported.future);
+        supported.future.complete(null);
         assertTrue(future.isDone());
     }
 
     @Test
     void noTestsResultInFailure() {
-        TestRunner runner = AggregateTestRunner.of(List.of(new MockTestRunner(false)));
-        assertEquals("No tests were found", runner.getLog(-1).iterator().next().getMessage());
-        assertSame(FAILURE, runner.getStatus());
+        TestRunner runner = AggregateTestRunner.of(List.of());
+        runner.test(null, null);
+        assertSame(NO_TESTS, runner.getStatus());
     }
 
     @Test
     void chainedRunners() {
         LogRecord record1 = new LogRecord(Level.INFO, "one");
         LogRecord record2 = new LogRecord(Level.INFO, "two");
-        MockTestRunner first = new MockTestRunner(true);
-        MockTestRunner second = new MockTestRunner(true);
+        MockTestRunner first = new MockTestRunner();
+        MockTestRunner second = new MockTestRunner();
         TestRunner runner = AggregateTestRunner.of(List.of(first, second));
         assertSame(NOT_STARTED, runner.getStatus());
         assertEquals(List.of(), runner.getLog(-1));
@@ -103,7 +108,7 @@ class AggregateTestRunnerTest {
         assertFalse(first.future.isDone());
         assertTrue(second.future.isDone());
         assertEquals(List.of(record1), runner.getLog(-1));
-        assertEquals(ERROR, runner.getStatus());
+        assertEquals(RUNNING, runner.getStatus());
 
         // First wrapped runner completes exceptionally, but the second should be started as usual.
         first.future.completeExceptionally(new RuntimeException("error"));
@@ -140,15 +145,10 @@ class AggregateTestRunnerTest {
     static class MockTestRunner implements TestRunner {
 
         final List<LogRecord> log = new ArrayList<>();
-        final boolean supported;
         CompletableFuture<?> future;
         Status status = NOT_STARTED;
         TestReport report;
 
-        public MockTestRunner(boolean supported) {
-            this.supported = supported;
-        }
-
         @Override
         public Collection<LogRecord> getLog(long after) {
             return log.stream().filter(record -> record.getSequenceNumber() > after).collect(toList());
@@ -165,11 +165,6 @@ class AggregateTestRunnerTest {
         }
 
         @Override
-        public boolean isSupported() {
-            return supported;
-        }
-
-        @Override
         public TestReport getReport() {
             return report;
         }
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 37ab8550357..9b08e398564 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
@@ -83,7 +83,6 @@ class TestRunnerHandlerTest {
     @Test
     public void returnsEmptyResponsesWhenReportNotReady() throws IOException {
         TestRunner testRunner = mock(TestRunner.class);
-        when(testRunner.isSupported()).thenReturn(true);
         when(testRunner.getReport()).thenReturn(null);
         testRunnerHandler = new TestRunnerHandler(
                 Executors.newSingleThreadExecutor(),
@@ -104,24 +103,6 @@ class TestRunnerHandlerTest {
         }
     }
 
-    @Test
-    public void usesLegacyTestRunnerWhenNotSupported() throws IOException {
-        TestRunner testRunner = mock(TestRunner.class);
-        when(testRunner.isSupported()).thenReturn(false);
-        TestRunner legacyTestRunner = mock(TestRunner.class);
-        when(legacyTestRunner.isSupported()).thenReturn(true);
-        when(legacyTestRunner.getLog(anyLong())).thenReturn(List.of(logRecord("Legacy log message")));
-        TestRunner aggregate = AggregateTestRunner.of(List.of(testRunner, legacyTestRunner));
-        testRunnerHandler = new TestRunnerHandler(Executors.newSingleThreadExecutor(), aggregate);
-
-        // Prime the aggregate to check for logs in the wrapped runners.
-        aggregate.test(TestRunner.Suite.PRODUCTION_TEST, new byte[0]);
-        HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/log", GET));
-        ByteArrayOutputStream out = new ByteArrayOutputStream();
-        response.render(out);
-        JsonTestHelper.assertJsonEquals(out.toString(UTF_8), "{\"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 results. */
     private static LogRecord logRecord(String logMessage) {
         LogRecord logRecord = new LogRecord(Level.INFO, logMessage);
@@ -154,11 +135,6 @@ class TestRunnerHandlerTest {
         }
 
         @Override
-        public boolean isSupported() {
-            return true;
-        }
-
-        @Override
         public TestRunner.Status getStatus() {
             return status;
         }
diff --git a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/VespaCliTestRunnerTest.java b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/VespaCliTestRunnerTest.java
index 46d7b78cdea..5fd73bb4494 100644
--- a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/VespaCliTestRunnerTest.java
+++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/VespaCliTestRunnerTest.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.testrunner;
 
 import ai.vespa.hosted.api.TestConfig;
-import com.yahoo.vespa.testrunner.VespaCliTestRunner.NoTestsException;
 import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
@@ -12,9 +11,7 @@ import java.nio.file.Path;
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
 /**
  * @author jonmv
@@ -48,13 +45,9 @@ class VespaCliTestRunnerTest {
         Path tests = Files.createDirectory(temp.resolve("tests"));
         Path artifacts = Files.createDirectory(temp.resolve("artifacts"));
         VespaCliTestRunner runner = new VespaCliTestRunner(artifacts, tests);
-        assertFalse(runner.isSupported());
 
         Path systemTests = Files.createDirectory(tests.resolve("system-test"));
-        assertTrue(runner.isSupported());
-        NoTestsException ise = assertThrows(NoTestsException.class,
-                                            () -> runner.testRunProcessBuilder(TestRunner.Suite.STAGING_TEST, testConfig));
-        assertEquals("No tests found, for suite 'STAGING_TEST'", ise.getMessage());
+        assertNull(runner.testRunProcessBuilder(TestRunner.Suite.STAGING_TEST, testConfig));
 
         ProcessBuilder builder = runner.testRunProcessBuilder(TestRunner.Suite.SYSTEM_TEST, testConfig);
         assertEquals(List.of("vespa", "test", systemTests.toAbsolutePath().toString(),