From a2d325add6496dca8e7c396b722bf4e14bb65393 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Wed, 17 Nov 2021 15:15:42 +0100 Subject: Revert "Merge pull request #20066 from vespa-engine/revert-20065-jonmv/vespa-cli-test-runner" This reverts commit e8e542a66a6bc9b2e33def248f208da63b7b934c, reversing changes made to 69bd22c6dea4bae7d210a9ea36b31ecd8cd5c7fa. --- .../controller/deployment/InternalStepRunner.java | 31 ++-- .../resources/test_runner_services.xml-cd-legacy | 2 +- .../resources/test_runner_services.xml-cd-osgi | 8 +- .../com/yahoo/vespa/testrunner/JunitRunner.java | 55 ++++--- .../com/yahoo/vespa/testrunner/TestReport.java | 66 +++----- .../com/yahoo/vespa/testrunner/TestRunner.java | 27 +++- .../yahoo/vespa/testrunner/TestRunnerHandler.java | 163 ++++++-------------- .../vespa/testrunner/legacy/LegacyTestRunner.java | 22 --- .../yahoo/vespa/testrunner/legacy/TestProfile.java | 30 ---- .../vespa/testrunner/TestRunnerHandlerTest.java | 57 ++++--- .../vespa/hosted/testrunner/PomXmlGenerator.java | 1 - .../yahoo/vespa/hosted/testrunner/TestProfile.java | 30 ++++ .../yahoo/vespa/hosted/testrunner/TestRunner.java | 18 ++- .../vespa/hosted/testrunner/TestRunnerHandler.java | 171 --------------------- .../hosted/testrunner/PomXmlGeneratorTest.java | 1 - .../hosted/testrunner/TestRunnerHandlerTest.java | 63 -------- .../vespa/hosted/testrunner/TestRunnerTest.java | 17 +- 17 files changed, 230 insertions(+), 532 deletions(-) delete mode 100644 vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/LegacyTestRunner.java delete mode 100644 vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/TestProfile.java create mode 100644 vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java delete mode 100644 vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java delete mode 100644 vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandlerTest.java diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 577dab69279..ddf89e2f376 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -919,24 +919,16 @@ public class InternalStepRunner implements StepRunner { String runtimeProviderClass = config.runtimeProviderClass(); String tenantCdBundle = config.tenantCdBundle(); - String handlerAndExtraComponents = useOsgiBasedTestRuntime - ? + String extraJUnitComponents = + "\n" + " \n" + - "\n" + - " \n" + - " \n" + - " artifacts\n" + - " " + systemUsesAthenz + "\n" + - " \n" + - " \n" + - "\n" + - " \n" + - " http://*/tester/v1/*\n" + - " \n" - : - " \n" + - " http://*/tester/v1/*\n" + - " \n"; + "\n" + + " \n" + + " \n" + + " artifacts\n" + + " " + systemUsesAthenz + "\n" + + " \n" + + " \n"; String servicesXml = "\n" + @@ -952,7 +944,10 @@ public class InternalStepRunner implements StepRunner { " \n" + " \n" + "\n" + - handlerAndExtraComponents + + " \n" + + " http://*/tester/v1/*\n" + + " \n" + + (useOsgiBasedTestRuntime ? extraJUnitComponents : "") + "\n" + " \n" + " " + resourceString + "\n" + diff --git a/controller-server/src/test/resources/test_runner_services.xml-cd-legacy b/controller-server/src/test/resources/test_runner_services.xml-cd-legacy index 125c5004d25..c6046479934 100644 --- a/controller-server/src/test/resources/test_runner_services.xml-cd-legacy +++ b/controller-server/src/test/resources/test_runner_services.xml-cd-legacy @@ -11,7 +11,7 @@ - + http://*/tester/v1/* diff --git a/controller-server/src/test/resources/test_runner_services.xml-cd-osgi b/controller-server/src/test/resources/test_runner_services.xml-cd-osgi index d1a83abff8d..01a7afb3bed 100644 --- a/controller-server/src/test/resources/test_runner_services.xml-cd-osgi +++ b/controller-server/src/test/resources/test_runner_services.xml-cd-osgi @@ -11,6 +11,10 @@ + + http://*/tester/v1/* + + @@ -20,10 +24,6 @@ - - http://*/tester/v1/* - - 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 87b98c8efc1..d1acc4faf0f 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 @@ -11,7 +11,6 @@ import com.yahoo.component.AbstractComponent; import com.yahoo.io.IOUtils; import com.yahoo.jdisc.application.OsgiFramework; import com.yahoo.vespa.defaults.Defaults; -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; @@ -26,10 +25,12 @@ import org.osgi.framework.BundleContext; import java.io.IOException; import java.net.URL; import java.nio.charset.Charset; -import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.SortedMap; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.logging.Level; @@ -44,6 +45,7 @@ import java.util.stream.Stream; public class JunitRunner extends AbstractComponent implements TestRunner { private static final Logger logger = Logger.getLogger(JunitRunner.class.getName()); + private final SortedMap logRecords = new ConcurrentSkipListMap<>(); private final BundleContext bundleContext; private final TestRuntimeProvider testRuntimeProvider; private volatile Future execution; @@ -84,12 +86,23 @@ public class JunitRunner extends AbstractComponent implements TestRunner { credentialsRoot.ifPresent(root -> System.setProperty("vespa.test.credentials.root", root)); } + private static TestDescriptor.TestCategory toCategory(TestRunner.Suite testProfile) { + switch(testProfile) { + case SYSTEM_TEST: return TestDescriptor.TestCategory.systemtest; + case STAGING_SETUP_TEST: return TestDescriptor.TestCategory.stagingsetuptest; + case STAGING_TEST: return TestDescriptor.TestCategory.stagingtest; + case PRODUCTION_TEST: return TestDescriptor.TestCategory.productiontest; + default: throw new RuntimeException("Unknown test profile: " + testProfile.name()); + } + } + @Override - public void executeTests(TestDescriptor.TestCategory category, byte[] testConfig) { - if (execution != null && !execution.isDone()) { + public void test(TestRunner.Suite suite, byte[] testConfig) { + if (execution != null && ! execution.isDone()) { throw new IllegalStateException("Test execution already in progress"); } try { + logRecords.clear(); testRuntimeProvider.initialize(testConfig); Optional testBundle = findTestBundle(); if (testBundle.isEmpty()) { @@ -100,12 +113,17 @@ public class JunitRunner extends AbstractComponent implements TestRunner { if (testDescriptor.isEmpty()) { throw new RuntimeException("Could not find test descriptor"); } - execution = CompletableFuture.supplyAsync(() -> launchJunit(loadClasses(testBundle.get(), testDescriptor.get(), category))); + execution = CompletableFuture.supplyAsync(() -> launchJunit(loadClasses(testBundle.get(), testDescriptor.get(), toCategory(suite)))); } catch (Exception e) { execution = CompletableFuture.completedFuture(createReportWithFailedInitialization(e)); } } + @Override + public Collection getLog(long after) { + return logRecords.tailMap(after + 1).values(); + } + private static TestReport createReportWithFailedInitialization(Exception exception) { TestReport.Failure failure = new TestReport.Failure("init", exception); return new TestReport.Builder() @@ -175,8 +193,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner { Launcher launcher = LauncherFactory.create(launcherConfig); // Create log listener: - var logLines = new ArrayList(); - var logListener = VespaJunitLogListener.forBiConsumer((t, m) -> log(logLines, m.get(), t)); + var logListener = VespaJunitLogListener.forBiConsumer((t, m) -> log(logRecords, m.get(), t)); // Create a summary listener: var summaryListener = new SummaryGeneratingListener(); launcher.registerTestExecutionListeners(logListener, summaryListener); @@ -193,14 +210,14 @@ public class JunitRunner extends AbstractComponent implements TestRunner { .withIgnoredCount(report.getTestsSkippedCount()) .withFailedCount(report.getTestsFailedCount()) .withFailures(failures) - .withLogs(logLines) + .withLogs(logRecords.values()) .build(); } - private void log(List logs, String message, Throwable t) { + private void log(SortedMap logs, String message, Throwable t) { LogRecord logRecord = new LogRecord(Level.INFO, message); Optional.ofNullable(t).ifPresent(logRecord::setThrown); - logs.add(logRecord); + logs.put(logRecord.getSequenceNumber(), logRecord); } @Override @@ -209,20 +226,20 @@ public class JunitRunner extends AbstractComponent implements TestRunner { } @Override - public LegacyTestRunner.Status getStatus() { - if (execution == null) return LegacyTestRunner.Status.NOT_STARTED; - if (!execution.isDone()) return LegacyTestRunner.Status.RUNNING; + public TestRunner.Status getStatus() { + if (execution == null) return TestRunner.Status.NOT_STARTED; + if (!execution.isDone()) return TestRunner.Status.RUNNING; try { TestReport report = execution.get(); if (report.isSuccess()) { - return LegacyTestRunner.Status.SUCCESS; + return TestRunner.Status.SUCCESS; } else { - return LegacyTestRunner.Status.FAILURE; + return TestRunner.Status.FAILURE; } } catch (InterruptedException|ExecutionException e) { logger.log(Level.WARNING, "Error while getting test report", e); // Return FAILURE to enforce getting the test report from the caller. - return LegacyTestRunner.Status.FAILURE; + return TestRunner.Status.FAILURE; } } @@ -242,10 +259,4 @@ public class JunitRunner extends AbstractComponent implements TestRunner { } } - @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 9a1200d0bf3..b3ca47e2480 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 @@ -1,13 +1,7 @@ // Copyright Yahoo. 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 java.nio.charset.StandardCharsets; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.logging.LogRecord; @@ -16,15 +10,16 @@ import java.util.logging.LogRecord; * @author mortent */ public class TestReport { - private final long totalCount; - private final long successCount; - private final long failedCount; - private final long ignoredCount; - private final long abortedCount; - private final List failures; - private final List logLines; - - public TestReport(long totalCount, long successCount, long failedCount, long ignoredCount, long abortedCount, List failures, List logLines) { + + final long totalCount; + final long successCount; + final long failedCount; + final long ignoredCount; + final long abortedCount; + final List failures; + final List logLines; + + private TestReport(long totalCount, long successCount, long failedCount, long ignoredCount, long abortedCount, List failures, List logLines) { this.totalCount = totalCount; this.successCount = successCount; this.failedCount = failedCount; @@ -34,31 +29,6 @@ public class TestReport { this.logLines = logLines; } - 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", 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(lr -> output.addString(lr.getMessage())); - - return Exceptions.uncheck(() -> new String(SlimeUtils.toJsonBytes(slime), StandardCharsets.UTF_8)); - } - public List logLines() { return logLines; } @@ -71,7 +41,9 @@ public class TestReport { return new Builder(); } + public static class Builder { + private long totalCount; private long successCount; private long failedCount; @@ -88,18 +60,22 @@ public class TestReport { 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; @@ -110,12 +86,14 @@ public class TestReport { return this; } - public Builder withLogs(List logRecords) { - this.logLines = logRecords; + public Builder withLogs(Collection logRecords) { + this.logLines = List.copyOf(logRecords); return this; } + } + public static class Failure { private final String testId; private final Throwable exception; @@ -132,5 +110,7 @@ public class TestReport { 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 index 31474d6c348..db489f5aa3d 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 @@ -1,20 +1,31 @@ // Copyright Yahoo. 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; +import java.util.Collection; +import java.util.logging.LogRecord; /** + * @author jonmv * @author mortent */ public interface TestRunner { - void executeTests(TestDescriptor.TestCategory category, byte[] testConfig); - boolean isSupported(); + Collection getLog(long after); - LegacyTestRunner.Status getStatus(); + Status getStatus(); - TestReport getReport(); + void test(Suite suite, byte[] config); - String getReportAsJson(); -} + default boolean isSupported() { return true; } + + default TestReport getReport() { return null; } + + enum Status { + NOT_STARTED, RUNNING, FAILURE, ERROR, SUCCESS + } + + enum Suite { + SYSTEM_TEST, STAGING_SETUP_TEST, STAGING_TEST, PRODUCTION_TEST + } + +} \ No newline at end of file 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 4c359071fc9..da2db3798c2 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 @@ -1,34 +1,26 @@ // Copyright Yahoo. 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.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; +import com.yahoo.exception.ExceptionUtils; +import com.yahoo.restapi.MessageResponse; +import com.yahoo.restapi.SlimeJsonResponse; 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; import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.OutputStream; import java.io.PrintStream; import java.util.Collection; -import java.util.Collections; -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; @@ -39,18 +31,12 @@ import static com.yahoo.jdisc.Response.Status; */ public class TestRunnerHandler extends LoggingRequestHandler { - private static final String CONTENT_TYPE_APPLICATION_JSON = "application/json"; - - private final TestRunner junitRunner; - private final LegacyTestRunner testRunner; - private final boolean useOsgiMode; + private final TestRunner testRunner; @Inject - public TestRunnerHandler(Executor executor, TestRunner junitRunner, LegacyTestRunner testRunner) { + public TestRunnerHandler(Executor executor, TestRunner junitRunner, TestRunner testRunner) { super(executor); - this.junitRunner = junitRunner; - this.testRunner = testRunner; - this.useOsgiMode = junitRunner.isSupported(); + this.testRunner = junitRunner.isSupported() ? junitRunner : testRunner; } @Override @@ -60,81 +46,48 @@ public class TestRunnerHandler extends LoggingRequestHandler { case GET: return handleGET(request); case POST: return handlePOST(request); - default: return new Response(Status.METHOD_NOT_ALLOWED, "Method '" + request.getMethod() + "' is not supported"); + default: return new MessageResponse(Status.METHOD_NOT_ALLOWED, "Method '" + request.getMethod() + "' is not supported"); } } catch (IllegalArgumentException e) { - return new Response(Status.BAD_REQUEST, Exceptions.toMessageString(e)); + return new MessageResponse(Status.BAD_REQUEST, Exceptions.toMessageString(e)); } catch (Exception e) { log.log(Level.WARNING, "Unexpected error handling '" + request.getUri() + "'", e); - return new Response(Status.INTERNAL_SERVER_ERROR, Exceptions.toMessageString(e)); + return new MessageResponse(Status.INTERNAL_SERVER_ERROR, Exceptions.toMessageString(e)); } } private HttpResponse handleGET(HttpRequest request) { String path = request.getUri().getPath(); - if (path.equals("/tester/v1/log")) { - if (useOsgiMode) { + switch (path) { + case "/tester/v1/log": long fetchRecordsAfter = Optional.ofNullable(request.getProperty("after")) - .map(Long::parseLong) - .orElse(-1L); - - List logRecords = Optional.ofNullable(junitRunner.getReport()) - .map(TestReport::logLines) - .orElse(Collections.emptyList()).stream() - .filter(record -> record.getSequenceNumber()>fetchRecordsAfter) - .collect(Collectors.toList()); - 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")) { - if (useOsgiMode) { - log.info("Responding with status " + junitRunner.getStatus()); - return new Response(junitRunner.getStatus().name()); - } else { + .map(Long::parseLong) + .orElse(-1L); + return new SlimeJsonResponse(logToSlime(testRunner.getLog(fetchRecordsAfter))); + case "/tester/v1/status": 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 MessageResponse(testRunner.getStatus().name()); + case "/tester/v1/report": + TestReport report = testRunner.getReport(); + if (report == null) + return new EmptyResponse(200); + + return new SlimeJsonResponse(toSlime(report)); } - return new Response(Status.NOT_FOUND, "Not found: " + request.getUri().getPath()); + return new MessageResponse(Status.NOT_FOUND, "Not found: " + request.getUri().getPath()); } private HttpResponse handlePOST(HttpRequest request) throws IOException { final String path = request.getUri().getPath(); if (path.startsWith("/tester/v1/run/")) { String type = lastElement(path); - TestProfile testProfile = TestProfile.valueOf(type.toUpperCase() + "_TEST"); + TestRunner.Suite testSuite = TestRunner.Suite.valueOf(type.toUpperCase() + "_TEST"); byte[] config = request.getData().readAllBytes(); - if (useOsgiMode) { - junitRunner.executeTests(categoryFromProfile(testProfile), 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()); - } - - TestDescriptor.TestCategory categoryFromProfile(TestProfile testProfile) { - switch(testProfile) { - case SYSTEM_TEST: return TestDescriptor.TestCategory.systemtest; - case STAGING_SETUP_TEST: return TestDescriptor.TestCategory.stagingsetuptest; - case STAGING_TEST: return TestDescriptor.TestCategory.stagingtest; - case PRODUCTION_TEST: return TestDescriptor.TestCategory.productiontest; - default: throw new RuntimeException("Unknown test profile: " + testProfile.name()); + testRunner.test(testSuite, config); + log.info("Started tests of type " + type + " and status is " + testRunner.getStatus()); + return new MessageResponse("Successfully started " + type + " tests"); } + return new MessageResponse(Status.NOT_FOUND, "Not found: " + request.getUri().getPath()); } private static String lastElement(String path) { @@ -177,48 +130,32 @@ public class TestRunnerHandler extends LoggingRequestHandler { : "error"; } - private static class SlimeJsonResponse extends HttpResponse { - private final Slime slime; + private static Slime toSlime(TestReport testReport) { + var slime = new Slime(); + var root = slime.setObject(); + if (testReport == null) + return slime; - private SlimeJsonResponse(Slime slime) { - super(200); - this.slime = 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); + summary.setLong("aborted", testReport.abortedCount); + var failureRoot = summary.setArray("failures"); + testReport.failures.forEach(failure -> serializeFailure(failure, failureRoot.addObject())); - @Override - public void render(OutputStream outputStream) throws IOException { - new JsonFormat(true).encode(outputStream, slime); - } + var output = root.setArray("output"); + testReport.logLines.forEach(lr -> output.addString(lr.getMessage())); - @Override - public String getContentType() { - return CONTENT_TYPE_APPLICATION_JSON; - } + return slime; } - private static class Response extends HttpResponse { - private static final ObjectMapper objectMapper = new ObjectMapper(); - private final String message; - - private Response(String response) { - this(200, response); - } - - private Response(int statusCode, String message) { - super(statusCode); - this.message = message; - } - - @Override - public void render(OutputStream outputStream) throws IOException { - ObjectNode objectNode = objectMapper.createObjectNode(); - objectNode.put("message", message); - objectMapper.writeValue(outputStream, objectNode); - } - - @Override - public String getContentType() { - return CONTENT_TYPE_APPLICATION_JSON; - } + private static void serializeFailure(TestReport.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())); } + } 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 deleted file mode 100644 index 418ab7fe5d0..00000000000 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/LegacyTestRunner.java +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.testrunner.legacy; - -import java.util.Collection; -import java.util.logging.LogRecord; - -/** - * @author mortent - */ -public interface LegacyTestRunner { - - Collection getLog(long after); - - Status getStatus(); - - 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 deleted file mode 100644 index ad65d150874..00000000000 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/TestProfile.java +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.testrunner.legacy; - -/** - * @author valerijf - * @author jvenstad - */ -public enum TestProfile { - - SYSTEM_TEST("system, com.yahoo.vespa.tenant.systemtest.base.SystemTest", true), - STAGING_SETUP_TEST("staging-setup", false), - STAGING_TEST("staging, com.yahoo.vespa.tenant.systemtest.base.StagingTest", true), - PRODUCTION_TEST("production, com.yahoo.vespa.tenant.systemtest.base.ProductionTest", false); - - private final String group; - private final boolean failIfNoTests; - - TestProfile(String group, boolean failIfNoTests) { - this.group = group; - this.failIfNoTests = failIfNoTests; - } - - public String group() { - return group; - } - - public boolean failIfNoTests() { - return failIfNoTests; - } -} 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 0caac95ed34..a78dc077446 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 @@ -1,24 +1,25 @@ // Copyright Yahoo. 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.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.Collection; import java.util.List; import java.util.concurrent.Executors; import java.util.logging.Level; import java.util.logging.LogRecord; +import java.util.stream.Collectors; import static com.yahoo.jdisc.http.HttpRequest.Method.GET; -import static org.junit.Assert.assertEquals; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -47,7 +48,7 @@ public class TestRunnerHandlerTest { testRunnerHandler = new TestRunnerHandler( Executors.newSingleThreadExecutor(), - new MockJunitRunner(LegacyTestRunner.Status.SUCCESS, testReport), + new MockJunitRunner(TestRunner.Status.SUCCESS, testReport), null); } @@ -56,7 +57,7 @@ public class TestRunnerHandlerTest { 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: but was:\",\"exception\":\"java.lang.RuntimeException: org.junit.ComparisonFailure: expected: but was:\\n\\tat Foo.bar(Foo.java:1123)\\n\"}]},\"output\":[\"Tests started\"]}"); + 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: but was:\",\"exception\":\"java.lang.RuntimeException: org.junit.ComparisonFailure: expected: but was:\\n\\tat Foo.bar(Foo.java:1123)\\n\"}]},\"output\":[\"Tests started\"]}"); } @@ -65,18 +66,18 @@ public class TestRunnerHandlerTest { 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\"}]}"); + JsonTestHelper.assertJsonEquals(out.toString(UTF_8), "{\"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=0", GET)); out = new ByteArrayOutputStream(); response.render(out); - assertEquals("{\"logRecords\":[]}", new String(out.toByteArray())); + assertEquals("{\"logRecords\":[]}", out.toString(UTF_8)); } @Test - public void returnsEmptyLogWhenReportNotReady() throws IOException { + public void returnsEmptyResponsesWhenReportNotReady() throws IOException { TestRunner testRunner = mock(TestRunner.class); when(testRunner.isSupported()).thenReturn(true); when(testRunner.getReport()).thenReturn(null); @@ -84,17 +85,26 @@ public class TestRunnerHandlerTest { Executors.newSingleThreadExecutor(), testRunner, null); - HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/log", GET)); - ByteArrayOutputStream out = new ByteArrayOutputStream(); - response.render(out); - assertEquals("{\"logRecords\":[]}", new String(out.toByteArray())); + { + HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/log", GET)); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + response.render(out); + assertEquals("{\"logRecords\":[]}", out.toString(UTF_8)); + } + + { + HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/report", GET)); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + response.render(out); + assertEquals("", out.toString(UTF_8)); + } } @Test public void usesLegacyTestRunnerWhenNotSupported() throws IOException { TestRunner testRunner = mock(TestRunner.class); when(testRunner.isSupported()).thenReturn(false); - LegacyTestRunner legacyTestRunner = mock(LegacyTestRunner.class); + TestRunner legacyTestRunner = mock(TestRunner.class); when(legacyTestRunner.getLog(anyLong())).thenReturn(List.of(logRecord("Legacy log message"))); testRunnerHandler = new TestRunnerHandler( @@ -104,7 +114,7 @@ public class TestRunnerHandlerTest { 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\"}]}"); + 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 format */ @@ -117,17 +127,23 @@ public class TestRunnerHandlerTest { private static class MockJunitRunner implements TestRunner { - private final LegacyTestRunner.Status status; + private final TestRunner.Status status; private final TestReport testReport; - public MockJunitRunner(LegacyTestRunner.Status status, TestReport testReport) { + public MockJunitRunner(TestRunner.Status status, TestReport testReport) { this.status = status; this.testReport = testReport; } @Override - public void executeTests(TestDescriptor.TestCategory category, byte[] testConfig) { + public void test(Suite suite, byte[] testConfig) { } + + @Override + public Collection getLog(long after) { + return getReport().logLines().stream() + .filter(entry -> entry.getSequenceNumber() > after) + .collect(Collectors.toList()); } @Override @@ -136,7 +152,7 @@ public class TestRunnerHandlerTest { } @Override - public LegacyTestRunner.Status getStatus() { + public TestRunner.Status getStatus() { return status; } @@ -145,9 +161,6 @@ public class TestRunnerHandlerTest { return testReport; } - @Override - public String getReportAsJson() { - return getReport().toJson(); - } } + } diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/PomXmlGenerator.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/PomXmlGenerator.java index b3c49fc6f2d..ff66b31dfa8 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/PomXmlGenerator.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/PomXmlGenerator.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.testrunner; import com.yahoo.vespa.defaults.Defaults; -import com.yahoo.vespa.testrunner.legacy.TestProfile; import java.nio.file.Path; import java.util.List; diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java new file mode 100644 index 00000000000..95a2b2723b8 --- /dev/null +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java @@ -0,0 +1,30 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.testrunner; + +/** + * @author valerijf + * @author jvenstad + */ +public enum TestProfile { + + SYSTEM_TEST("system, com.yahoo.vespa.tenant.systemtest.base.SystemTest", true), + STAGING_SETUP_TEST("staging-setup", false), + STAGING_TEST("staging, com.yahoo.vespa.tenant.systemtest.base.StagingTest", true), + PRODUCTION_TEST("production, com.yahoo.vespa.tenant.systemtest.base.ProductionTest", false); + + private final String group; + private final boolean failIfNoTests; + + TestProfile(String group, boolean failIfNoTests) { + this.group = group; + this.failIfNoTests = failIfNoTests; + } + + public String group() { + return group; + } + + public boolean failIfNoTests() { + return failIfNoTests; + } +} diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java index 0f6e26d256f..8aa2cd7bc72 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java @@ -3,8 +3,6 @@ package com.yahoo.vespa.hosted.testrunner; import com.google.inject.Inject; import com.yahoo.vespa.defaults.Defaults; -import com.yahoo.vespa.testrunner.legacy.LegacyTestRunner; -import com.yahoo.vespa.testrunner.legacy.TestProfile; import org.fusesource.jansi.AnsiOutputStream; import org.fusesource.jansi.HtmlAnsiOutputStream; @@ -38,7 +36,7 @@ import static java.util.logging.Level.SEVERE; * @author valerijf * @author jvenstad */ -public class TestRunner implements LegacyTestRunner { +public class TestRunner implements com.yahoo.vespa.testrunner.TestRunner { private static final Logger logger = Logger.getLogger(TestRunner.class.getName()); private static final Level HTML = new Level("html", 1) { }; @@ -114,14 +112,14 @@ public class TestRunner implements LegacyTestRunner { return builder; } - public synchronized void test(TestProfile testProfile, byte[] testConfig) { + public synchronized void test(Suite suite, byte[] testConfig) { if (status == Status.RUNNING) throw new IllegalArgumentException("Tests are already running; should not receive this request now."); log.clear(); status = Status.RUNNING; - new Thread(() -> runTests(testProfile, testConfig)).start(); + new Thread(() -> runTests(toProfile(suite), testConfig)).start(); } public Collection getLog(long after) { @@ -210,4 +208,14 @@ public class TestRunner implements LegacyTestRunner { private NoTestsException(String message) { super(message); } } + static TestProfile toProfile(Suite suite) { + switch (suite) { + case SYSTEM_TEST: return TestProfile.SYSTEM_TEST; + case STAGING_SETUP_TEST: return TestProfile.STAGING_SETUP_TEST; + case STAGING_TEST: return TestProfile.STAGING_TEST; + case PRODUCTION_TEST: return TestProfile.PRODUCTION_TEST; + default: throw new IllegalArgumentException("Unknown test suite '" + suite + "'"); + } + } + } diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java deleted file mode 100644 index 621f0964a40..00000000000 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java +++ /dev/null @@ -1,171 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.testrunner; - -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.google.inject.Inject; -import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.container.jdisc.LoggingRequestHandler; -import com.yahoo.io.IOUtils; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.JsonFormat; -import com.yahoo.slime.Slime; -import com.yahoo.vespa.testrunner.legacy.TestProfile; -import com.yahoo.yolean.Exceptions; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.OutputStream; -import java.io.PrintStream; -import java.util.Collection; -import java.util.concurrent.Executor; -import java.util.logging.Level; -import java.util.logging.LogRecord; - -import static com.yahoo.jdisc.Response.Status; - -/** - * @author valerijf - * @author jvenstad - */ -public class TestRunnerHandler extends LoggingRequestHandler { - - private static final String CONTENT_TYPE_APPLICATION_JSON = "application/json"; - - private final TestRunner testRunner; - - @Inject - public TestRunnerHandler(Executor executor, TestRunner testRunner) { - super(executor); - this.testRunner = testRunner; - } - - @Override - public HttpResponse handle(HttpRequest request) { - try { - switch (request.getMethod()) { - case GET: return handleGET(request); - case POST: return handlePOST(request); - - default: return new Response(Status.METHOD_NOT_ALLOWED, "Method '" + request.getMethod() + "' is not supported"); - } - } catch (IllegalArgumentException e) { - return new Response(Status.BAD_REQUEST, Exceptions.toMessageString(e)); - } catch (Exception e) { - log.log(Level.WARNING, "Unexpected error handling '" + request.getUri() + "'", e); - return new Response(Status.INTERNAL_SERVER_ERROR, Exceptions.toMessageString(e)); - } - } - - 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))); - } else if (path.equals("/tester/v1/status")) { - log.info("Responding with status " + testRunner.getStatus()); - return new Response(testRunner.getStatus().name()); - } - return new Response(Status.NOT_FOUND, "Not found: " + request.getUri().getPath()); - } - - private HttpResponse handlePOST(HttpRequest request) throws IOException { - final String path = request.getUri().getPath(); - if (path.startsWith("/tester/v1/run/")) { - String type = lastElement(path); - TestProfile testProfile = TestProfile.valueOf(type.toUpperCase() + "_TEST"); - byte[] config = IOUtils.readBytes(request.getData(), 1 << 16); - 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()); - } - - private static String lastElement(String path) { - if (path.endsWith("/")) - path = path.substring(0, path.length() - 1); - int lastSlash = path.lastIndexOf("/"); - if (lastSlash < 0) return path; - return path.substring(lastSlash + 1); - } - - static Slime logToSlime(Collection log) { - Slime slime = new Slime(); - Cursor root = slime.setObject(); - Cursor recordArray = root.setArray("logRecords"); - logArrayToSlime(recordArray, log); - return slime; - } - - static void logArrayToSlime(Cursor recordArray, Collection log) { - log.forEach(record -> { - Cursor recordObject = recordArray.addObject(); - recordObject.setLong("id", record.getSequenceNumber()); - recordObject.setLong("at", record.getMillis()); - recordObject.setString("type", typeOf(record.getLevel())); - String message = record.getMessage(); - if (record.getThrown() != null) { - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); - record.getThrown().printStackTrace(new PrintStream(buffer)); - message += "\n" + buffer; - } - recordObject.setString("message", message); - }); - } - - public static String typeOf(Level level) { - return level.getName().equals("html") ? "html" - : level.intValue() < Level.INFO.intValue() ? "debug" - : level.intValue() < Level.WARNING.intValue() ? "info" - : level.intValue() < Level.SEVERE.intValue() ? "warning" - : "error"; - } - - private static class SlimeJsonResponse extends HttpResponse { - private final Slime slime; - - private SlimeJsonResponse(Slime slime) { - super(200); - this.slime = slime; - } - - @Override - public void render(OutputStream outputStream) throws IOException { - new JsonFormat(true).encode(outputStream, slime); - } - - @Override - public String getContentType() { - return CONTENT_TYPE_APPLICATION_JSON; - } - } - - private static class Response extends HttpResponse { - private static final ObjectMapper objectMapper = new ObjectMapper(); - private final String message; - - private Response(String response) { - this(200, response); - } - - private Response(int statusCode, String message) { - super(statusCode); - this.message = message; - } - - @Override - public void render(OutputStream outputStream) throws IOException { - ObjectNode objectNode = objectMapper.createObjectNode(); - objectNode.put("message", message); - objectMapper.writeValue(outputStream, objectNode); - } - - @Override - public String getContentType() { - return CONTENT_TYPE_APPLICATION_JSON; - } - } -} diff --git a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/PomXmlGeneratorTest.java b/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/PomXmlGeneratorTest.java index 391e2a0abbf..943583ae42b 100644 --- a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/PomXmlGeneratorTest.java +++ b/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/PomXmlGeneratorTest.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.testrunner; -import com.yahoo.vespa.testrunner.legacy.TestProfile; import org.junit.Test; import java.io.IOException; diff --git a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandlerTest.java b/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandlerTest.java deleted file mode 100644 index fdc6b633630..00000000000 --- a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandlerTest.java +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.testrunner; - -import com.yahoo.slime.SlimeUtils; -import org.junit.Test; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.PrintStream; -import java.time.Instant; -import java.util.Collections; -import java.util.logging.Level; -import java.util.logging.LogRecord; - -import static org.junit.Assert.assertEquals; - -/** - * @author jvenstad - */ -public class TestRunnerHandlerTest { - - @Test - public void logSerialization() throws IOException { - Log log = new Log(); - LogRecord record = log.getLogRecord(); - String trace = log.getTrace(); - assertEquals("{\"logRecords\":[{\"id\":1,\"at\":2,\"type\":\"info\",\"message\":\"Hello.\\n" + trace + "\"}]}", - new String(SlimeUtils.toJsonBytes(TestRunnerHandler.logToSlime(Collections.singletonList(record))))); - } - - private static class Log { - - private final LogRecord record; - private final String trace; - - public Log() { - Exception exception = new RuntimeException(); - record = createRecord(exception); - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); - exception.printStackTrace(new PrintStream(buffer)); - trace = buffer.toString() - .replaceAll("\n", "\\\\n") - .replaceAll("\t", "\\\\t"); - } - - LogRecord getLogRecord() { - return record; - } - - String getTrace() { - return trace; - } - - private static LogRecord createRecord(Exception exception) { - LogRecord record = new LogRecord(Level.INFO, "Hello."); - record.setSequenceNumber(1); - record.setInstant(Instant.ofEpochMilli(2)); - record.setThrown(exception); - return record; - } - } - -} diff --git a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerTest.java b/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerTest.java index 4612f5b217a..b513dfba8b5 100644 --- a/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerTest.java +++ b/vespa-testrunner-components/src/test/java/com/yahoo/vespa/hosted/testrunner/TestRunnerTest.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.testrunner; -import com.yahoo.vespa.testrunner.legacy.TestProfile; import org.fusesource.jansi.Ansi; import org.junit.Before; import org.junit.Rule; @@ -14,6 +13,8 @@ import java.nio.file.Path; import java.util.Iterator; import java.util.logging.LogRecord; +import static com.yahoo.vespa.testrunner.TestRunner.Suite.STAGING_TEST; +import static com.yahoo.vespa.testrunner.TestRunner.Suite.SYSTEM_TEST; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -48,7 +49,7 @@ public class TestRunnerTest { public void ansiCodesAreConvertedToHtml() throws InterruptedException { TestRunner runner = new TestRunner(artifactsPath, testPath, logFile, configFile, settingsFile, __ -> new ProcessBuilder("echo", Ansi.ansi().fg(Ansi.Color.RED).a("Hello!").reset().toString())); - runner.test(TestProfile.SYSTEM_TEST, new byte[0]); + runner.test(SYSTEM_TEST, new byte[0]); while (runner.getStatus() == TestRunner.Status.RUNNING) { Thread.sleep(10); } @@ -65,7 +66,7 @@ public class TestRunnerTest { Files.delete(artifactsPath.resolve("my-tests.jar")); TestRunner runner = new TestRunner(artifactsPath, testPath, logFile, configFile, settingsFile, __ -> new ProcessBuilder("This is a command that doesn't exist, for sure!")); - runner.test(TestProfile.SYSTEM_TEST, new byte[0]); + runner.test(SYSTEM_TEST, new byte[0]); while (runner.getStatus() == TestRunner.Status.RUNNING) { Thread.sleep(10); } @@ -81,7 +82,7 @@ public class TestRunnerTest { public void errorLeadsToError() throws InterruptedException { TestRunner runner = new TestRunner(artifactsPath, testPath, logFile, configFile, settingsFile, __ -> new ProcessBuilder("false")); - runner.test(TestProfile.SYSTEM_TEST, new byte[0]); + runner.test(SYSTEM_TEST, new byte[0]); while (runner.getStatus() == TestRunner.Status.RUNNING) { Thread.sleep(10); } @@ -93,7 +94,7 @@ public class TestRunnerTest { public void failureLeadsToFailure() throws InterruptedException { TestRunner runner = new TestRunner(artifactsPath, testPath, logFile, configFile, settingsFile, __ -> new ProcessBuilder("false")); - runner.test(TestProfile.SYSTEM_TEST, new byte[0]); + runner.test(SYSTEM_TEST, new byte[0]); while (runner.getStatus() == TestRunner.Status.RUNNING) { Thread.sleep(10); } @@ -105,7 +106,7 @@ public class TestRunnerTest { public void filesAreGenerated() throws InterruptedException, IOException { TestRunner runner = new TestRunner(artifactsPath, testPath, logFile, configFile, settingsFile, __ -> new ProcessBuilder("echo", "Hello!")); - runner.test(TestProfile.SYSTEM_TEST, "config".getBytes()); + runner.test(SYSTEM_TEST, "config".getBytes()); while (runner.getStatus() == TestRunner.Status.RUNNING) { Thread.sleep(10); } @@ -119,7 +120,7 @@ public class TestRunnerTest { public void runnerCanBeReused() throws InterruptedException, IOException { TestRunner runner = new TestRunner(artifactsPath, testPath, logFile, configFile, settingsFile, __ -> new ProcessBuilder("sleep", "0.1")); - runner.test(TestProfile.SYSTEM_TEST, "config".getBytes()); + runner.test(SYSTEM_TEST, "config".getBytes()); assertEquals(TestRunner.Status.RUNNING, runner.getStatus()); while (runner.getStatus() == TestRunner.Status.RUNNING) { @@ -128,7 +129,7 @@ public class TestRunnerTest { assertEquals(1, runner.getLog(-1).size()); assertEquals(TestRunner.Status.SUCCESS, runner.getStatus()); - runner.test(TestProfile.STAGING_TEST, "newConfig".getBytes()); + runner.test(STAGING_TEST, "newConfig".getBytes()); while (runner.getStatus() == TestRunner.Status.RUNNING) { Thread.sleep(10); } -- cgit v1.2.3 From 7cddf841ac8774ecb1d359cac7e1c65059314733 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Wed, 17 Nov 2021 15:48:32 +0100 Subject: Always read test report, and propagate correct status --- .../src/main/java/com/yahoo/component/provider/ComponentRegistry.java | 2 +- .../yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java | 1 + .../src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java | 3 +-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/component/src/main/java/com/yahoo/component/provider/ComponentRegistry.java b/component/src/main/java/com/yahoo/component/provider/ComponentRegistry.java index ec6f21e4f53..1dddcbcc461 100644 --- a/component/src/main/java/com/yahoo/component/provider/ComponentRegistry.java +++ b/component/src/main/java/com/yahoo/component/provider/ComponentRegistry.java @@ -20,7 +20,7 @@ import java.util.Set; *

* This registry supports the freeze pattern - changes can be made * to this registry until {@link #freeze} is called. Subsequent change attempts will cause an - * exception. Freezing a registry after building makes it possible toi avoid locking and memory + * exception. Freezing a registry after building makes it possible to avoid locking and memory * synchronization on lookups. * * @author bratseth diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index ddf89e2f376..8563375ab5c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -640,6 +640,7 @@ public class InternalStepRunner implements StepRunner { return Optional.of(testFailure); case ERROR: logger.log(INFO, "Tester failed running its tests!"); + controller.jobController().updateTestReport(id); return Optional.of(error); case SUCCESS: logger.log("Tests completed successfully."); 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 d1acc4faf0f..d0f3b879fec 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 @@ -238,8 +238,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner { } } catch (InterruptedException|ExecutionException e) { logger.log(Level.WARNING, "Error while getting test report", e); - // Return FAILURE to enforce getting the test report from the caller. - return TestRunner.Status.FAILURE; + return TestRunner.Status.ERROR; } } -- cgit v1.2.3 From fbdc8549df12ffd68f470bdbabff72d49951cf61 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Wed, 17 Nov 2021 19:28:27 +0100 Subject: Have registry injected, and wrap runners in an aggregate --- .../vespa/testrunner/AggregateTestRunner.java | 119 ++++++++++++++ .../com/yahoo/vespa/testrunner/JunitRunner.java | 6 +- .../com/yahoo/vespa/testrunner/TestRunner.java | 5 +- .../yahoo/vespa/testrunner/TestRunnerHandler.java | 10 +- .../vespa/testrunner/legacy/package-info.java | 9 -- .../vespa/testrunner/AggregateTestRunnerTest.java | 174 +++++++++++++++++++++ .../vespa/testrunner/TestRunnerHandlerTest.java | 43 ++--- .../yahoo/vespa/hosted/testrunner/TestRunner.java | 13 +- 8 files changed, 342 insertions(+), 37 deletions(-) create mode 100644 vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/AggregateTestRunner.java delete mode 100644 vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/package-info.java create mode 100644 vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java 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 new file mode 100644 index 00000000000..82c1f7194d0 --- /dev/null +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/AggregateTestRunner.java @@ -0,0 +1,119 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.testrunner; + +import java.util.ArrayList; +import java.util.Collection; +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 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 wrapped; + private final AtomicInteger current = new AtomicInteger(-1); + + private AggregateTestRunner(List testRunners) { + this.wrapped = testRunners; + } + + public static TestRunner of(Collection testRunners) { + List supported = testRunners.stream().filter(TestRunner::isSupported).collect(toUnmodifiableList()); + return supported.isEmpty() ? noRunner : new AggregateTestRunner(supported); + } + + @Override + public Collection getLog(long after) { + ArrayList records = new ArrayList<>(); + for (int i = 0; i <= current.get() && i < wrapped.size(); i++) + records.addAll(wrapped.get(i).getLog(after)); + + return records; + } + + @Override + public Status getStatus() { + if (current.get() == -1) + return Status.NOT_STARTED; + + boolean failed = false; + for (int i = 0; i <= current.get(); i++) { + if (i == wrapped.size()) + return failed ? Status.FAILURE : Status.SUCCESS; + + switch (wrapped.get(i).getStatus()) { + case ERROR: return Status.ERROR; + case FAILURE: failed = true; + } + } + return Status.RUNNING; + } + + @Override + public CompletableFuture test(Suite suite, byte[] config) { + if (0 <= current.get() && current.get() < wrapped.size()) + throw new IllegalStateException("Tests already running, should not attempt to start now"); + + current.set(-1); + CompletableFuture aggregate = new CompletableFuture<>(); + CompletableFuture vessel = CompletableFuture.completedFuture(null); + runNext(suite, config, vessel, aggregate); + return aggregate; + } + + private void runNext(Suite suite, byte[] config, CompletableFuture vessel, CompletableFuture aggregate) { + vessel.whenComplete((__, ___) -> { + int next = current.incrementAndGet(); + if (next == wrapped.size()) + aggregate.complete(null); + else + runNext(suite, config, wrapped.get(next).test(suite, config), aggregate); + }); + } + + @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); + } + + static TestReport merge(TestReport first, TestReport second) { + return TestReport.builder() + .withAbortedCount(first.abortedCount + second.abortedCount) + .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(); + } + + static List merged(List first, List second) { + ArrayList merged = new ArrayList<>(); + merged.addAll(first); + merged.addAll(second); + return merged; + } + +} 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 d0f3b879fec..6aa36c62416 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 @@ -32,7 +32,6 @@ import java.util.SortedMap; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentSkipListMap; 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; @@ -48,7 +47,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner { private final SortedMap logRecords = new ConcurrentSkipListMap<>(); private final BundleContext bundleContext; private final TestRuntimeProvider testRuntimeProvider; - private volatile Future execution; + private volatile CompletableFuture execution; @Inject public JunitRunner(OsgiFramework osgiFramework, @@ -97,7 +96,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner { } @Override - public void test(TestRunner.Suite suite, byte[] testConfig) { + public CompletableFuture test(Suite suite, byte[] testConfig) { if (execution != null && ! execution.isDone()) { throw new IllegalStateException("Test execution already in progress"); } @@ -117,6 +116,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner { } catch (Exception e) { execution = CompletableFuture.completedFuture(createReportWithFailedInitialization(e)); } + return execution; } @Override 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 db489f5aa3d..d70a3f60c7d 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.testrunner; import java.util.Collection; +import java.util.concurrent.CompletableFuture; import java.util.logging.LogRecord; /** @@ -14,9 +15,9 @@ public interface TestRunner { Status getStatus(); - void test(Suite suite, byte[] config); + CompletableFuture test(Suite suite, byte[] config); - default boolean isSupported() { return true; } + boolean isSupported(); default TestReport getReport() { return null; } 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 da2db3798c2..62601e4dfa0 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.testrunner; import com.google.inject.Inject; +import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.jdisc.EmptyResponse; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; @@ -34,9 +35,13 @@ public class TestRunnerHandler extends LoggingRequestHandler { private final TestRunner testRunner; @Inject - public TestRunnerHandler(Executor executor, TestRunner junitRunner, TestRunner testRunner) { + public TestRunnerHandler(Executor executor, ComponentRegistry testRunners) { + this(executor, AggregateTestRunner.of(testRunners.allComponents())); + } + + TestRunnerHandler(Executor executor, TestRunner testRunner) { super(executor); - this.testRunner = junitRunner.isSupported() ? junitRunner : testRunner; + this.testRunner = testRunner; } @Override @@ -152,7 +157,6 @@ public class TestRunnerHandler extends LoggingRequestHandler { } private static void serializeFailure(TestReport.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())); diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/package-info.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/package-info.java deleted file mode 100644 index 6f6a8c819a6..00000000000 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/legacy/package-info.java +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -/** - * @author mortent - */ -@ExportPackage -package com.yahoo.vespa.testrunner.legacy; - -import com.yahoo.osgi.annotation.ExportPackage; \ No newline at end of file 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 new file mode 100644 index 00000000000..64f9079643a --- /dev/null +++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java @@ -0,0 +1,174 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.testrunner; + +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.logging.Level; +import java.util.logging.LogRecord; + +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.NOT_STARTED; +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; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * @author jonmv + */ +class AggregateTestRunnerTest { + + @Test + void onlySupportedRunnersAreUsed() { + MockTestRunner unsupported = new MockTestRunner(false); + MockTestRunner suppported = new MockTestRunner(true); + TestRunner runner = AggregateTestRunner.of(List.of(unsupported, suppported)); + CompletableFuture future = runner.test(null, null); + assertFalse(future.isDone()); + assertNull(unsupported.future); + assertNotNull(suppported.future); + suppported.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()); + } + + @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); + TestRunner runner = AggregateTestRunner.of(List.of(first, second)); + assertSame(NOT_STARTED, runner.getStatus()); + assertEquals(List.of(), runner.getLog(-1)); + + // First wrapped runner is started. + CompletableFuture future = runner.test(null, null); + assertNotNull(first.future); + assertNull(second.future); + assertEquals(RUNNING, runner.getStatus()); + + // Logs from first wrapped runner are returned. + assertEquals(List.of(), runner.getLog(-1)); + first.log.add(record1); + assertEquals(List.of(record1), runner.getLog(-1)); + assertEquals(List.of(), runner.getLog(record1.getSequenceNumber())); + + // First wrapped runner completes, second is started. + first.status = SUCCESS; + first.future.complete(null); + assertNotNull(second.future); + assertFalse(future.isDone()); + assertEquals(RUNNING, runner.getStatus()); + + // Logs from second runner are available. + second.log.add(record2); + assertEquals(List.of(record1, record2), runner.getLog(-1)); + + // No failures means success. + second.future.complete(null); + assertEquals(SUCCESS, runner.getStatus()); + + // A failure means failure. + second.status = FAILURE; + assertEquals(FAILURE, runner.getStatus()); + + // An error means error. + first.status = ERROR; + assertEquals(ERROR, runner.getStatus()); + + // Runner is re-used. Ensure nothing from the second wrapped runner is visible. + runner.test(null, null); + assertFalse(first.future.isDone()); + assertTrue(second.future.isDone()); + assertEquals(List.of(record1), runner.getLog(-1)); + assertEquals(ERROR, runner.getStatus()); + + // First wrapped runner completes exceptionally, but the second should be started as usual. + first.future.completeExceptionally(new RuntimeException("error")); + assertFalse(second.future.isDone()); + assertEquals(List.of(record1, record2), runner.getLog(-1)); + + // Verify reports are merged. + assertNull(runner.getReport()); + + TestReport.Failure failure = new TestReport.Failure("test", null); + TestReport report = TestReport.builder() + .withLogs(List.of(record1)) + .withFailures(List.of(failure)) + .withTotalCount(15) + .withSuccessCount(8) + .withIgnoredCount(4) + .withFailedCount(2) + .withAbortedCount(1) + .build(); + first.report = report; + assertSame(report, runner.getReport()); + + second.report = report; + 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); + } + + static class MockTestRunner implements TestRunner { + + final List log = new ArrayList<>(); + final boolean supported; + CompletableFuture future; + Status status = NOT_STARTED; + TestReport report; + + public MockTestRunner(boolean supported) { + this.supported = supported; + } + + @Override + public Collection getLog(long after) { + return log.stream().filter(record -> record.getSequenceNumber() > after).collect(toList()); + } + + @Override + public Status getStatus() { + return status; + } + + @Override + public CompletableFuture test(Suite suite, byte[] config) { + return future = new CompletableFuture<>(); + } + + @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 a78dc077446..37ab8550357 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 @@ -1,10 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.testrunner; +import com.yahoo.component.ComponentId; +import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.test.json.JsonTestHelper; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.io.ByteArrayOutputStream; @@ -12,6 +14,7 @@ import java.io.IOException; import java.time.Instant; import java.util.Collection; import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executors; import java.util.logging.Level; import java.util.logging.LogRecord; @@ -27,13 +30,15 @@ import static org.mockito.Mockito.when; /** * @author mortent */ -public class TestRunnerHandlerTest { +class TestRunnerHandlerTest { private static final Instant testInstant = Instant.ofEpochMilli(1598432151660L); - private static TestRunnerHandler testRunnerHandler; - @BeforeAll - public static void setup() { + private TestRunnerHandler testRunnerHandler; + private TestRunner aggregateRunner; + + @BeforeEach + void setup() { List logRecords = List.of(logRecord("Tests started")); Throwable exception = new RuntimeException("org.junit.ComparisonFailure: expected: but was:"); exception.setStackTrace(new StackTraceElement[]{new StackTraceElement("Foo", "bar", "Foo.java", 1123)}); @@ -46,10 +51,8 @@ public class TestRunnerHandlerTest { .withFailures(List.of(new TestReport.Failure("Foo.bar()", exception))) .withLogs(logRecords).build(); - testRunnerHandler = new TestRunnerHandler( - Executors.newSingleThreadExecutor(), - new MockJunitRunner(TestRunner.Status.SUCCESS, testReport), - null); + aggregateRunner = AggregateTestRunner.of(List.of(new MockJunitRunner(TestRunner.Status.SUCCESS, testReport))); + testRunnerHandler = new TestRunnerHandler(Executors.newSingleThreadExecutor(), aggregateRunner); } @Test @@ -58,11 +61,13 @@ public class TestRunnerHandlerTest { 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: but was:\",\"exception\":\"java.lang.RuntimeException: org.junit.ComparisonFailure: expected: but was:\\n\\tat Foo.bar(Foo.java:1123)\\n\"}]},\"output\":[\"Tests started\"]}"); - } @Test public void returnsCorrectLog() throws IOException { + // Prime the aggregate runner to actually consider the wrapped runner for logs. + aggregateRunner.test(TestRunner.Suite.SYSTEM_TEST, new byte[0]); + HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/log", GET)); ByteArrayOutputStream out = new ByteArrayOutputStream(); response.render(out); @@ -73,7 +78,6 @@ public class TestRunnerHandlerTest { out = new ByteArrayOutputStream(); response.render(out); assertEquals("{\"logRecords\":[]}", out.toString(UTF_8)); - } @Test @@ -83,7 +87,7 @@ public class TestRunnerHandlerTest { when(testRunner.getReport()).thenReturn(null); testRunnerHandler = new TestRunnerHandler( Executors.newSingleThreadExecutor(), - testRunner, null); + ComponentRegistry.singleton(new ComponentId("runner"), testRunner)); { HttpResponse response = testRunnerHandler.handle(HttpRequest.createTestRequest("http://localhost:1234/tester/v1/log", GET)); @@ -105,19 +109,20 @@ public class TestRunnerHandlerTest { 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); - testRunnerHandler = new TestRunnerHandler( - Executors.newSingleThreadExecutor(), - testRunner, legacyTestRunner); - + // 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 format */ + /* 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); logRecord.setInstant(testInstant); @@ -137,7 +142,9 @@ public class TestRunnerHandlerTest { } @Override - public void test(Suite suite, byte[] testConfig) { } + public CompletableFuture test(Suite suite, byte[] testConfig) { + return CompletableFuture.completedFuture(null); + } @Override public Collection getLog(long after) { diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java index 8aa2cd7bc72..06f7d317b0e 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.SortedMap; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentSkipListMap; import java.util.function.Function; import java.util.logging.Level; @@ -112,24 +113,32 @@ public class TestRunner implements com.yahoo.vespa.testrunner.TestRunner { return builder; } - public synchronized void test(Suite suite, byte[] testConfig) { + @Override + public synchronized CompletableFuture test(Suite suite, byte[] testConfig) { if (status == Status.RUNNING) throw new IllegalArgumentException("Tests are already running; should not receive this request now."); log.clear(); status = Status.RUNNING; - new Thread(() -> runTests(toProfile(suite), testConfig)).start(); + return CompletableFuture.runAsync(() -> runTests(toProfile(suite), testConfig)); } + @Override public Collection getLog(long after) { return log.tailMap(after + 1).values(); } + @Override public synchronized Status getStatus() { return status; } + @Override + public boolean isSupported() { + return listFiles(artifactsPath).stream().anyMatch(file -> file.toString().endsWith("tests.jar")); + } + private void runTests(TestProfile testProfile, byte[] testConfig) { ProcessBuilder builder = testBuilder.apply(testProfile); { -- cgit v1.2.3