Merge pull request #23626 from vespa-engine/jonmv/tests-cleanup

Jonmv/tests cleanup

4 files changed, 12 insertions, 131 deletions

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 38d7b6d3a2b..ef3474e0c1e 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
@@ -656,10 +656,13 @@ public class InternalStepRunner implements StepRunner {
                 controller.jobController().updateTestReport(id);
                 return Optional.of(testFailure);
             case INCONCLUSIVE:
-                long sleepMinutes = Math.max(15, Math.min(120, Duration.between(deployment.get().at(), controller.clock().instant()).toMinutes() / 20));
-                logger.log("Tests were inconclusive, and will run again in " + sleepMinutes + " minutes.");
                 controller.jobController().updateTestReport(id);
-                controller.jobController().locked(id, run -> run.sleepingUntil(controller.clock().instant().plusSeconds(60 * sleepMinutes)));
+                controller.jobController().locked(id, run -> {
+                    Instant nextAttemptAt = run.start();
+                    while ( ! nextAttemptAt.isAfter(controller.clock().instant())) nextAttemptAt = nextAttemptAt.plusSeconds(1800);
+                    logger.log("Tests were inconclusive, and will run again at " + nextAttemptAt + ".");
+                    return run.sleepingUntil(nextAttemptAt);
+                });
                 return Optional.of(reset);
             case ERROR:
                 logger.log(INFO, "Tester failed running its tests!");
@@ -806,6 +809,7 @@ public class InternalStepRunner implements StepRunner {
         Consumer<String> updater = msg -> controller.notificationsDb().setNotification(source, Notification.Type.deployment, Notification.Level.error, msg);
         switch (run.status()) {
             case aborted: return; // wait and see how the next run goes.
+            case noTests:
             case running:
             case success:
                 controller.notificationsDb().removeNotification(source, Notification.Type.deployment);
@@ -822,10 +826,6 @@ public class InternalStepRunner implements StepRunner {
             case testFailure:
                 updater.accept("one or more verification tests against the deployment failed. Please review test output in the deployment job log.");
                 return;
-            case noTests:
-                controller.notificationsDb().setNotification(source, Notification.Type.deployment, Notification.Level.warning,
-                                                             "no tests were found for this job type. Please review test output in the deployment job log.");
-                return;
             case error:
             case endpointCertificateTimeout:
                 break;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index bf488198126..493c0945ecc 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -358,7 +358,8 @@ public class InternalStepRunnerTest {
         // Test sleeps for a while.
         tester.runner().run();
         assertEquals(unfinished, tester.jobs().run(id).stepStatuses().get(Step.deployTester));
-        tester.clock().advance(Duration.ofSeconds(899));
+        Instant nextAttemptAt = tester.clock().instant().plusSeconds(1800);
+        tester.clock().advance(Duration.ofSeconds(1799));
         tester.runner().run();
         assertEquals(unfinished, tester.jobs().run(id).stepStatuses().get(Step.deployTester));
 
@@ -380,8 +381,8 @@ public class InternalStepRunnerTest {
 
         assertTestLogEntries(id, Step.endTests,
                              new LogEntry(lastId1 + 1, Instant.ofEpochMilli(123), info, "Not enough data!"),
-                             new LogEntry(lastId1 + 2, instant1, info, "Tests were inconclusive, and will run again in 15 minutes."),
-                             new LogEntry(lastId1 + 15, instant1, info, "### Run will reset, and start over at " + instant1.plusSeconds(900).truncatedTo(SECONDS)),
+                             new LogEntry(lastId1 + 2, instant1, info, "Tests were inconclusive, and will run again at " + nextAttemptAt + "."),
+                             new LogEntry(lastId1 + 15, instant1, info, "### Run will reset, and start over at " + nextAttemptAt),
                              new LogEntry(lastId1 + 16, instant1, info, ""),
                              new LogEntry(lastId2 + 1, tester.clock().instant(), info, "Tests completed successfully."));
 
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 ef4b402d33b..756c3f55ab3 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
@@ -150,61 +150,10 @@ public class TestRunnerHandler extends ThreadedHttpRequestHandler {
         json.writeFieldName("report");
         render(json, (Node) report.root());
 
-        // TODO jonmv: remove
-        json.writeObjectFieldStart("summary");
-
-        renderSummary(json, report);
-
-        json.writeArrayFieldStart("failures");
-        renderFailures(json, report.root());
-        json.writeEndArray();
-
-        json.writeEndObject();
-
-        // TODO jonmv: remove
-        json.writeArrayFieldStart("output");
-        renderOutput(json, report.root());
-        json.writeEndArray();
-
         json.writeEndObject();
         json.close();
     }
 
-    private static void renderSummary(JsonGenerator json, TestReport report) throws IOException {
-        Map<TestReport.Status, Long> tally =  report.root().tally();
-        json.writeNumberField("success", tally.getOrDefault(TestReport.Status.successful, 0L));
-        json.writeNumberField("failed", tally.getOrDefault(TestReport.Status.failed, 0L) + tally.getOrDefault(TestReport.Status.error, 0L));
-        json.writeNumberField("ignored", tally.getOrDefault(TestReport.Status.skipped, 0L));
-        json.writeNumberField("aborted", tally.getOrDefault(TestReport.Status.aborted, 0L));
-        json.writeNumberField("inconclusive", tally.getOrDefault(TestReport.Status.inconclusive, 0L));
-    }
-
-    private static void renderFailures(JsonGenerator json, Node node) throws IOException {
-        if (node instanceof FailureNode) {
-            json.writeStartObject();
-            json.writeStringField("testName", node.parent.name());
-            json.writeStringField("testError", ((FailureNode) node).thrown().getMessage());
-            json.writeStringField("exception", ExceptionUtils.getStackTraceAsString(((FailureNode) node).thrown()));
-            json.writeEndObject();
-        }
-        else {
-            for (Node child : node.children())
-                renderFailures(json, child);
-        }
-    }
-
-    private static void renderOutput(JsonGenerator json, Node node) throws IOException {
-        if (node instanceof OutputNode) {
-            for (LogRecord record : ((OutputNode) node).log())
-                if (record.getMessage() != null)
-                    json.writeString(formatter.format(record.getInstant().atOffset(ZoneOffset.UTC)) + " " + record.getMessage());
-        }
-        else {
-            for (Node child : node.children())
-                renderOutput(json, child);
-        }
-    }
-
     private static void render(JsonGenerator json, Node node) throws IOException {
         json.writeStartObject();
         if (node instanceof NamedNode) render(json, (NamedNode) node);
diff --git a/vespa-osgi-testrunner/src/test/resources/report.json b/vespa-osgi-testrunner/src/test/resources/report.json
index 443694e2e0c..66ae6dd398c 100644
--- a/vespa-osgi-testrunner/src/test/resources/report.json
+++ b/vespa-osgi-testrunner/src/test/resources/report.json
@@ -482,74 +482,5 @@
         ]
       }
     ]
-  },
-  "summary": {
-    "success": 3,
-    "failed": 5,
-    "ignored": 4,
-    "aborted": 1,
-    "inconclusive": 1,
-    "failures": [
-      {
-        "testName": "error()",
-        "testError": null,
-        "exception": "java.lang.NoClassDefFoundError\n\tat com.yahoo.vespa.test.samples.SampleTest.error(SampleTest.java:87)\n"
-      },
-      {
-        "testName": "failing()",
-        "testError": "baz ==> expected: <foo> but was: <bar>",
-        "exception": "org.opentest4j.AssertionFailedError: baz ==> expected: <foo> but was: <bar>\n\tat org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)\n\tat org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)\n\tat org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)\n\tat org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1152)\n\tat com.yahoo.vespa.test.samples.SampleTest.failing(SampleTest.java:81)\n"
-      },
-      {
-        "testName": "inconclusive(TestReporter)",
-        "testError": "the cat is both dead _and_ alive",
-        "exception": "ai.vespa.hosted.cd.InconclusiveTestException: the cat is both dead _and_ alive\n\tat com.yahoo.vespa.test.samples.SampleTest.inconclusive(SampleTest.java:93)\n"
-      },
-      {
-        "testName": "third",
-        "testError": "no charm",
-        "exception": "org.opentest4j.AssertionFailedError: no charm\n\tat org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:39)\n\tat org.junit.jupiter.api.Assertions.fail(Assertions.java:134)\n\tat com.yahoo.vespa.test.samples.SampleTest$Inner.lambda$others$1(SampleTest.java:105)\n"
-      },
-      {
-        "testName": "test()",
-        "testError": "",
-        "exception": "org.opentest4j.AssertionFailedError\n\tat org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:35)\n\tat org.junit.jupiter.api.Assertions.fail(Assertions.java:115)\n\tat com.yahoo.vespa.test.samples.FailingTestAndBothAftersTest.test(FailingTestAndBothAftersTest.java:19)\n\tSuppressed: java.lang.RuntimeException\n\t\tat com.yahoo.vespa.test.samples.FailingTestAndBothAftersTest.moreFail(FailingTestAndBothAftersTest.java:16)\n"
-      },
-      {
-        "testName": "FailingTestAndBothAftersTest",
-        "testError": null,
-        "exception": "java.lang.RuntimeException\n\tat com.yahoo.vespa.test.samples.FailingTestAndBothAftersTest.fail(FailingTestAndBothAftersTest.java:13)\n"
-      },
-      {
-        "testName": "WrongBeforeAllTest",
-        "testError": "@BeforeAll method 'void com.yahoo.vespa.test.samples.WrongBeforeAllTest.wrong()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).",
-        "exception": "org.junit.platform.commons.JUnitException: @BeforeAll method 'void com.yahoo.vespa.test.samples.WrongBeforeAllTest.wrong()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).\n"
-      },
-      {
-        "testName": "test()",
-        "testError": null,
-        "exception": "java.lang.NullPointerException\n\tat com.yahoo.vespa.test.samples.FailingExtensionTest$FailingExtension.<init>(FailingExtensionTest.java:19)\n"
-      },
-      {
-        "testName": "Production test",
-        "testError": "School's out all summer!",
-        "exception": "java.lang.ClassNotFoundException: School's out all summer!\n"
-      }
-    ]
-  },
-  "output": [
-    "00:00:00.000 spam",
-    "00:00:00.000 spam",
-    "00:00:00.000 spam",
-    "00:00:00.000 I have a bad feeling about this",
-    "00:00:00.000 spam",
-    "00:00:00.000 I'm here with Erwin today; Erwin, what can you tell us about your cat?",
-    "00:00:00.000 spam",
-    "00:00:00.000 <body />",
-    "00:00:00.000 Very informative: \"\\n\": \n",
-    "00:00:00.000 Oh no",
-    "00:00:00.000 spam",
-    "00:00:00.000 Catch me if you can!",
-    "00:00:00.000 spam"
-  ]
+  }
 }