Use jackson JsonGenerator for test output JSON

5 files changed, 104 insertions, 121 deletions

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 aa51971583b..ef4b402d33b 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,6 +1,8 @@
 // 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.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.core.JsonGenerator;
 import com.yahoo.component.annotation.Inject;
 import com.yahoo.component.provider.ComponentRegistry;
 import com.yahoo.container.jdisc.EmptyResponse;
@@ -40,6 +42,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 public class TestRunnerHandler extends ThreadedHttpRequestHandler {
 
     private static final DateTimeFormatter formatter = DateTimeFormatter.ofPattern("HH:mm:ss.SSS");
+    private static final JsonFactory factory = new JsonFactory();
 
     private final TestRunner testRunner;
 
@@ -109,9 +112,10 @@ public class TestRunnerHandler extends ThreadedHttpRequestHandler {
         return path.substring(lastSlash + 1);
     }
 
-    static void render(OutputStream out, Collection<LogRecord> log) throws IOException {
-        out.write("{\"logRecords\":[".getBytes(UTF_8));
-        boolean first = true;
+    private static void render(OutputStream out, Collection<LogRecord> log) throws IOException {
+        var json = factory.createGenerator(out);
+        json.writeStartObject();
+        json.writeArrayFieldStart("logRecords");
         for (LogRecord record : log) {
             String message = record.getMessage() == null ? "" : record.getMessage();
             if (record.getThrown() != null) {
@@ -119,20 +123,19 @@ public class TestRunnerHandler extends ThreadedHttpRequestHandler {
                 record.getThrown().printStackTrace(new PrintStream(buffer));
                 message += (message.isEmpty() ? "" : "\n") + buffer;
             }
-
-            if (first) first = false;
-            else out.write(',');
-            out.write("""
-                      {"id":%d,"at":%d,"type":"%s","message":"%s"}
-                      """.formatted(record.getSequenceNumber(),
-                                    record.getMillis(),
-                                    typeOf(record.getLevel()),
-                                    message).getBytes(UTF_8));
+            json.writeStartObject();
+            json.writeNumberField("id", record.getSequenceNumber());
+            json.writeNumberField("at", record.getMillis());
+            json.writeStringField("type", typeOf(record.getLevel()));
+            json.writeStringField("message", message);
+            json.writeEndObject();
         }
-        out.write("]}".getBytes(UTF_8));
+        json.writeEndArray();
+        json.writeEndObject();
+        json.close();
     }
 
-    public static String typeOf(Level level) {
+    private static String typeOf(Level level) {
         return    level.getName().equals("html") ? "html"
                 : level.intValue() < Level.INFO.intValue() ? "debug"
                 : level.intValue() < Level.WARNING.intValue() ? "info"
@@ -140,116 +143,105 @@ public class TestRunnerHandler extends ThreadedHttpRequestHandler {
                 : "error";
     }
 
-    static void render(OutputStream out, TestReport report) throws IOException {
-        out.write('{');
+    private static void render(OutputStream out, TestReport report) throws IOException {
+        JsonGenerator json = factory.createGenerator(out);
+        json.writeStartObject();
 
-        out.write("\"report\":".getBytes(UTF_8));
-        render(out, (Node) report.root());
+        json.writeFieldName("report");
+        render(json, (Node) report.root());
 
         // TODO jonmv: remove
-        out.write(",\"summary\":{".getBytes(UTF_8));
+        json.writeObjectFieldStart("summary");
 
-        renderSummary(out, report);
+        renderSummary(json, report);
 
-        out.write(",\"failures\":[".getBytes(UTF_8));
-        renderFailures(out, report.root(), true);
-        out.write("]".getBytes(UTF_8));
+        json.writeArrayFieldStart("failures");
+        renderFailures(json, report.root());
+        json.writeEndArray();
 
-        out.write("}".getBytes(UTF_8));
+        json.writeEndObject();
 
         // TODO jonmv: remove
-        out.write(",\"output\":[".getBytes(UTF_8));
-        renderOutput(out, report.root(), true);
-        out.write("]".getBytes(UTF_8));
+        json.writeArrayFieldStart("output");
+        renderOutput(json, report.root());
+        json.writeEndArray();
 
-        out.write('}');
+        json.writeEndObject();
+        json.close();
     }
 
-    static void renderSummary(OutputStream out, TestReport report) throws IOException {
+    private static void renderSummary(JsonGenerator json, TestReport report) throws IOException {
         Map<TestReport.Status, Long> tally =  report.root().tally();
-        out.write("""
-                  "success":%d,"failed":%d,"ignored":%d,"aborted":%d,"inconclusive":%d
-                  """.formatted(tally.getOrDefault(TestReport.Status.successful, 0L),
-                                tally.getOrDefault(TestReport.Status.failed, 0L) + tally.getOrDefault(TestReport.Status.error, 0L),
-                                tally.getOrDefault(TestReport.Status.skipped, 0L),
-                                tally.getOrDefault(TestReport.Status.aborted, 0L),
-                                tally.getOrDefault(TestReport.Status.inconclusive, 0L)).getBytes(UTF_8));
+        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));
     }
 
-    static boolean renderFailures(OutputStream out, Node node, boolean first) throws IOException {
+    private static void renderFailures(JsonGenerator json, Node node) throws IOException {
         if (node instanceof FailureNode) {
-            if (first) first = false;
-            else out.write(',');
-            String message = ((FailureNode) node).thrown().getMessage();
-            out.write("""
-                      {"testName":"%s","testError":%s,"exception":"%s"}
-                      """.formatted(node.parent.name(),
-                                    message == null ? null : '"' + message + '"',
-                                    ExceptionUtils.getStackTraceAsString(((FailureNode) node).thrown())).getBytes(UTF_8));
+            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())
-                first = renderFailures(out, child, first);
+                renderFailures(json, child);
         }
-        return first;
     }
 
-    static boolean renderOutput(OutputStream out, Node node, boolean first) throws IOException {
+    private static void renderOutput(JsonGenerator json, Node node) throws IOException {
         if (node instanceof OutputNode) {
             for (LogRecord record : ((OutputNode) node).log())
-                if (record.getMessage() != null) {
-                    if (first) first = false;
-                    else out.write(',');
-                    out.write(('"' + formatter.format(record.getInstant().atOffset(ZoneOffset.UTC)) + " " + record.getMessage() + '"').getBytes(UTF_8));
-                }
+                if (record.getMessage() != null)
+                    json.writeString(formatter.format(record.getInstant().atOffset(ZoneOffset.UTC)) + " " + record.getMessage());
         }
         else {
             for (Node child : node.children())
-                first = renderOutput(out, child, first);
+                renderOutput(json, child);
         }
-        return first;
     }
 
-    static void render(OutputStream out, Node node) throws IOException {
-        out.write('{');
-        if (node instanceof NamedNode) render(out, (NamedNode) node);
-        if (node instanceof OutputNode) render(out, (OutputNode) node);
+    private static void render(JsonGenerator json, Node node) throws IOException {
+        json.writeStartObject();
+        if (node instanceof NamedNode) render(json, (NamedNode) node);
+        if (node instanceof OutputNode) render(json, (OutputNode) node);
 
         if ( ! node.children().isEmpty()) {
-            out.write(",\"children\":[".getBytes(UTF_8));
-            boolean first = true;
+            json.writeArrayFieldStart("children");
             for (Node child : node.children) {
-                if (first) first = false;
-                else out.write(',');
-                render(out, child);
+                render(json, child);
             }
-            out.write(']');
+            json.writeEndArray();
         }
-        out.write('}');
+        json.writeEndObject();
     }
 
-    static void render(OutputStream out, NamedNode node) throws IOException {
+    private static void render(JsonGenerator json, NamedNode node) throws IOException {
         String type = node instanceof FailureNode ? "failure" : node instanceof TestNode ? "test" : "container";
-        out.write("""
-                  "type":"%s","name":"%s","status":"%s","start":%d,"duration":%d
-                  """.formatted(type,node.name(), node.status().name(), node.start().toEpochMilli(), node.duration().toMillis()).getBytes(UTF_8));
+        json.writeStringField("type", type);
+        json.writeStringField("name", node.name());
+        json.writeStringField("status", node.status().name());
+        json.writeNumberField("start", node.start().toEpochMilli());
+        json.writeNumberField("duration", node.duration().toMillis());
     }
 
-    static void render(OutputStream out, OutputNode node) throws IOException {
-        out.write("\"type\":\"output\",\"children\":[".getBytes(UTF_8));
-        boolean first = true;
+    private static void render(JsonGenerator json, OutputNode node) throws IOException {
+        json.writeStringField("type", "output");
+        json.writeArrayFieldStart("children");
         for (LogRecord record : node.log()) {
-            if (first) first = false;
-            else out.write(',');
-            out.write("""
-                      {"message":"%s","at":%d,"level":"%s"}
-                      """.formatted((record.getLoggerName() == null ? "" : record.getLoggerName() + ": ") +
-                                    (record.getMessage() != null ? record.getMessage() : "") +
-                                    (record.getThrown() != null ? (record.getMessage() != null ? "\n" : "") + traceToString(record.getThrown()) : ""),
-                                    record.getInstant().toEpochMilli(),
-                                    typeOf(record.getLevel())).getBytes(UTF_8));
+            json.writeStartObject();
+            json.writeStringField("message", (record.getLoggerName() == null ? "" : record.getLoggerName() + ": ") +
+                                             (record.getMessage() != null ? record.getMessage() : "") +
+                                             (record.getThrown() != null ? (record.getMessage() != null ? "\n" : "") + traceToString(record.getThrown()) : ""));
+            json.writeNumberField("at", record.getInstant().toEpochMilli());
+            json.writeStringField("level", typeOf(record.getLevel()));
+            json.writeEndObject();
         }
-        out.write(']');
+        json.writeEndArray();
     }
 
     private static String traceToString(Throwable thrown) {
@@ -258,17 +250,17 @@ public class TestRunnerHandler extends ThreadedHttpRequestHandler {
         return buffer.toString(UTF_8);
     }
 
-    interface Renderer {
+    private interface Renderer {
 
         void render(OutputStream out) throws IOException;
 
     }
 
-    static class CustomJsonResponse extends HttpResponse {
+    private static class CustomJsonResponse extends HttpResponse {
 
         private final Renderer renderer;
 
-        CustomJsonResponse(Renderer renderer) {
+        private CustomJsonResponse(Renderer renderer) {
             super(200);
             this.renderer = renderer;
         }
diff --git a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/test/samples/SampleTest.java b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/test/samples/SampleTest.java
index bc878353d4b..b0e5119c06e 100644
--- a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/test/samples/SampleTest.java
+++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/test/samples/SampleTest.java
@@ -71,7 +71,7 @@ public class SampleTest {
     @Test
     void successful() {
         log.log(new Level("html", INFO.intValue()) { }, "<body />");
-        log.log(INFO, "Very informative");
+        log.log(INFO, "Very informative: \"\\n\": \n");
         log.log(WARNING, "Oh no", new IllegalArgumentException("error", new RuntimeException("wrapped")));
     }
 
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 5ce737d7649..6d6fbbf2cf1 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
@@ -27,7 +27,6 @@ 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;
 
 import static com.yahoo.jdisc.http.HttpRequest.Method.GET;
@@ -136,14 +135,6 @@ class TestRunnerHandlerTest {
         }
     }
 
-    /* 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);
-        logRecord.setSequenceNumber(0);
-        return logRecord;
-    }
-
     private static class MockRunner implements TestRunner {
 
         private final TestRunner.Status status;
diff --git a/vespa-osgi-testrunner/src/test/resources/output.json b/vespa-osgi-testrunner/src/test/resources/output.json
index 847ae5800e9..2b4aa9e5599 100644
--- a/vespa-osgi-testrunner/src/test/resources/output.json
+++ b/vespa-osgi-testrunner/src/test/resources/output.json
@@ -1,133 +1,133 @@
 {
   "logRecords": [
     {
-      "id": 18,
+      "id": 2,
       "at": 0,
       "type": "info",
       "message": "spam"
     },
     {
-      "id": 21,
+      "id": 5,
       "at": 0,
       "type": "info",
       "message": "spam"
     },
     {
-      "id": 22,
+      "id": 6,
       "at": 0,
       "type": "error",
       "message": "java.lang.NoClassDefFoundError\n\tat com.yahoo.vespa.test.samples.SampleTest.error(SampleTest.java:87)\n"
     },
     {
-      "id": 25,
+      "id": 9,
       "at": 0,
       "type": "info",
       "message": "spam"
     },
     {
-      "id": 26,
+      "id": 10,
       "at": 0,
       "type": "info",
       "message": "I have a bad feeling about this"
     },
     {
-      "id": 27,
+      "id": 11,
       "at": 0,
       "type": "error",
       "message": "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"
     },
     {
-      "id": 31,
+      "id": 15,
       "at": 0,
       "type": "info",
       "message": "spam"
     },
     {
-      "id": 32,
+      "id": 16,
       "at": 0,
       "type": "info",
       "message": "I'm here with Erwin today; Erwin, what can you tell us about your cat?"
     },
     {
-      "id": 33,
+      "id": 17,
       "at": 0,
       "type": "warning",
       "message": "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"
     },
     {
-      "id": 36,
+      "id": 20,
       "at": 0,
       "type": "info",
       "message": "spam"
     },
     {
-      "id": 37,
+      "id": 21,
       "at": 0,
       "type": "info",
       "message": "<body />"
     },
     {
-      "id": 38,
+      "id": 22,
       "at": 0,
       "type": "info",
-      "message": "Very informative"
+      "message": "Very informative: \"\\n\": \n"
     },
     {
-      "id": 39,
+      "id": 23,
       "at": 0,
       "type": "warning",
       "message": "Oh no\njava.lang.IllegalArgumentException: error\n\tat com.yahoo.vespa.test.samples.SampleTest.successful(SampleTest.java:75)\nCaused by: java.lang.RuntimeException: wrapped\n\t... 1 more\n"
     },
     {
-      "id": 43,
+      "id": 27,
       "at": 0,
       "type": "info",
       "message": "spam"
     },
     {
-      "id": 46,
+      "id": 30,
       "at": 0,
       "type": "info",
       "message": "Catch me if you can!"
     },
     {
-      "id": 50,
+      "id": 34,
       "at": 0,
       "type": "error",
       "message": "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"
     },
     {
-      "id": 54,
+      "id": 38,
       "at": 0,
       "type": "info",
       "message": "spam"
     },
     {
-      "id": 2,
+      "id": 67,
       "at": 0,
       "type": "error",
       "message": "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"
     },
     {
-      "id": 4,
+      "id": 69,
       "at": 0,
       "type": "error",
       "message": "java.lang.RuntimeException\n\tat com.yahoo.vespa.test.samples.FailingTestAndBothAftersTest.fail(FailingTestAndBothAftersTest.java:13)\n"
     },
     {
-      "id": 7,
+      "id": 72,
       "at": 0,
       "type": "error",
       "message": "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"
     },
     {
-      "id": 11,
+      "id": 76,
       "at": 0,
       "type": "error",
       "message": "java.lang.NullPointerException\n\tat com.yahoo.vespa.test.samples.FailingExtensionTest$FailingExtension.<init>(FailingExtensionTest.java:19)\n"
     },
     {
-      "id": 15,
+      "id": 80,
       "at": 12000,
       "type": "error",
       "message": "java.lang.ClassNotFoundException: School's out all summer!\n"
diff --git a/vespa-osgi-testrunner/src/test/resources/report.json b/vespa-osgi-testrunner/src/test/resources/report.json
index fa76c222f93..443694e2e0c 100644
--- a/vespa-osgi-testrunner/src/test/resources/report.json
+++ b/vespa-osgi-testrunner/src/test/resources/report.json
@@ -191,7 +191,7 @@
                         "level": "info"
                       },
                       {
-                        "message": "com.yahoo.vespa.test.samples.SampleTest: Very informative",
+                        "message": "com.yahoo.vespa.test.samples.SampleTest: Very informative: \"\\n\": \n",
                         "at": 0,
                         "level": "info"
                       },
@@ -546,7 +546,7 @@
     "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",
+    "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!",