diff --git a/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporter.java b/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporter.java deleted file mode 100644 index 5ab9da0e..00000000 --- a/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/FormattingTestReporter.java +++ /dev/null @@ -1,175 +0,0 @@ -/* - * (c) Copyright 2021 Palantir Technologies Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.palantir.witchcraft.java.logging.gradle.testreport; - -// CHECKSTYLE:OFF - -import com.palantir.witchcraft.java.logging.format.LogFormatter; -import com.palantir.witchcraft.java.logging.format.LogParser; -import java.io.File; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.io.Writer; -import java.util.function.BiConsumer; -import org.gradle.api.Action; -import org.gradle.api.internal.tasks.testing.junit.result.TestClassResult; -import org.gradle.api.internal.tasks.testing.junit.result.TestResultsProvider; -import org.gradle.api.internal.tasks.testing.report.HtmlTestReport; -import org.gradle.api.internal.tasks.testing.report.TestReporter; -import org.gradle.api.tasks.testing.TestOutputEvent; -// CHECKSTYLE:ON - -final class FormattingTestReporter implements TestReporter { - - private final Object delegate; - - FormattingTestReporter(Object delegate) { - this.delegate = delegate; - } - - @Override - public void generateReport(TestResultsProvider testResultsProvider, File file) { - if (delegate instanceof TestReporter) { - ((TestReporter) delegate).generateReport(new FormattingTestResultsProvider(testResultsProvider), file); - } else if (delegate instanceof HtmlTestReport) { - ((HtmlTestReport) delegate).generateReport(new FormattingTestResultsProvider(testResultsProvider), file); - } else { - throw new IllegalArgumentException("Unknown delegate class: " + delegate.getClass()); - } - } - - private static final class FormattingTestResultsProvider implements TestResultsProvider { - - private static final LogParser PARSER = new LogParser<>(TestLogFilter.INSTANCE.combineWith( - LogFormatter.INSTANCE, - (include, formatted) -> include - ? writer -> { - writer.write(formatted); - writer.write('\n'); - } - : Writable.NOP)); - private static final BiConsumer LINE_PROCESSOR = (line, outputWriter) -> { - try { - PARSER.tryParse(line) - .orElseGet(() -> out -> { - try { - out.write(line); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }) - .write(outputWriter); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }; - - private final TestResultsProvider delegate; - - FormattingTestResultsProvider(TestResultsProvider delegate) { - this.delegate = delegate; - } - - @Override - public void writeAllOutput(long classId, TestOutputEvent.Destination destination, Writer writer) { - if (destination == TestOutputEvent.Destination.StdErr - || destination == TestOutputEvent.Destination.StdOut) { - delegate.writeAllOutput(classId, destination, new LineProcessingWriter(writer, LINE_PROCESSOR)); - } else { - delegate.writeAllOutput(classId, destination, writer); - } - } - - private static class LineProcessingWriter extends Writer { - private final Writer delegate; - private final BiConsumer lineProcessor; - private final StringBuilder lineBuffer = new StringBuilder(); - - LineProcessingWriter(Writer delegate, BiConsumer lineProcessor) { - this.delegate = delegate; - this.lineProcessor = lineProcessor; - } - - @Override - public void write(char[] cbuf, int off, int len) throws IOException { - for (int i = off; i < off + len; i++) { - char ch = cbuf[i]; - if (ch == '\n') { - processLine(); - } else { - lineBuffer.append(ch); - } - } - } - - @Override - public void flush() throws IOException { - delegate.flush(); - } - - @Override - public void close() throws IOException { - if (!lineBuffer.isEmpty()) { - processLine(); - } - delegate.close(); - } - - private void processLine() throws IOException { - String line = lineBuffer.toString(); - lineProcessor.accept(line, delegate); - delegate.write('\n'); - lineBuffer.setLength(0); - } - } - - @Override - public void writeNonTestOutput(long classId, TestOutputEvent.Destination destination, Writer writer) { - delegate.writeNonTestOutput(classId, destination, writer); - } - - @Override - public void writeTestOutput(long classId, long testId, TestOutputEvent.Destination destination, Writer writer) { - delegate.writeTestOutput(classId, testId, destination, writer); - } - - @Override - public void visitClasses(Action visitor) { - delegate.visitClasses(visitor); - } - - @Override - public boolean hasOutput(long classId, TestOutputEvent.Destination destination) { - return delegate.hasOutput(classId, destination); - } - - @Override - public boolean hasOutput(long classId, long testId, TestOutputEvent.Destination destination) { - return delegate.hasOutput(classId, testId, destination); - } - - @Override - public boolean isHasResults() { - return delegate.isHasResults(); - } - - @Override - public void close() throws IOException { - delegate.close(); - } - } -} diff --git a/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/HtmlReportPostProcessor.java b/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/HtmlReportPostProcessor.java new file mode 100644 index 00000000..bce10164 --- /dev/null +++ b/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/HtmlReportPostProcessor.java @@ -0,0 +1,125 @@ +/* + * (c) Copyright 2021 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.witchcraft.java.logging.gradle.testreport; + +import com.palantir.witchcraft.java.logging.format.LogFormatter; +import com.palantir.witchcraft.java.logging.format.LogParser; +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * Post-processes HTML test report files to apply witchcraft log formatting. + */ +final class HtmlReportPostProcessor { + + private static final LogParser> PARSER = new LogParser<>(TestLogFilter.INSTANCE.combineWith( + LogFormatter.INSTANCE, (include, formatted) -> include ? Optional.of(formatted) : Optional.empty())); + + // Match 
 tags within stdout/stderr sections: 
standard output
...
...

+    private static final Pattern OUTPUT_SECTION_PATTERN = Pattern.compile(
+            "(
standard (?:output|error)
\\s*]*>\\s*]*>)(.*?)(
)", + Pattern.DOTALL | Pattern.CASE_INSENSITIVE); + + void processReportDirectory(File reportDir) { + Optional.ofNullable(reportDir).filter(File::isDirectory).ifPresent(this::walkAndProcessHtmlFiles); + } + + private void walkAndProcessHtmlFiles(File reportDir) { + try (Stream paths = Files.walk(reportDir.toPath())) { + paths.filter(path -> path.toString().endsWith(".html")).forEach(this::processHtmlFile); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private void processHtmlFile(Path htmlFile) { + try { + String content = Files.readString(htmlFile, StandardCharsets.UTF_8); + String processed = processHtmlContent(content); + + if (!content.equals(processed)) { + Files.writeString(htmlFile, processed, StandardCharsets.UTF_8); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + String processHtmlContent(String html) { + Matcher matcher = OUTPUT_SECTION_PATTERN.matcher(html); + StringBuilder result = new StringBuilder(); + + while (matcher.find()) { + String replacement = matcher.group(1) + formatPreContent(matcher.group(2)) + matcher.group(3); + matcher.appendReplacement(result, Matcher.quoteReplacement(replacement)); + } + matcher.appendTail(result); + + return result.toString(); + } + + private String formatPreContent(String content) { + return Arrays.stream(content.split("\n")) + .map(this::formatLine) + .flatMap(Optional::stream) + .collect(Collectors.joining("\n")); + } + + /** + * @return formatted line (with trailing newline), empty if filtered out, + * or original if not a witchcraft log + */ + private Optional formatLine(String line) { + String decoded = htmlDecode(line); + + return PARSER.tryParse(decoded) + .map(opt -> opt.map(formatted -> htmlEncode(formatted) + "\n")) + .orElse(Optional.of(line)); + } + + private static String htmlDecode(String html) { + return html.replace("<", "<") + .replace(">", ">") + .replace("&", "&") + .replace(""", "\"") + .replace("'", "'") + .replace("'", "'"); + } + + private static String htmlEncode(String text) { + return text.chars() + .mapToObj(c -> switch (c) { + case '&' -> "&"; + case '<' -> "<"; + case '>' -> ">"; + case '"' -> """; + case '\'' -> "'"; + default -> String.valueOf((char) c); + }) + .collect(Collectors.joining()); + } +} diff --git a/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPlugin.java b/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPlugin.java index 68454645..ce1b466c 100644 --- a/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPlugin.java +++ b/gradle-witchcraft-logging/src/main/groovy/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPlugin.java @@ -16,96 +16,53 @@ package com.palantir.witchcraft.java.logging.gradle.testreport; -// CHECKSTYLE:OFF - -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; +import java.io.File; +import java.util.Optional; import javax.inject.Inject; import org.gradle.api.Plugin; import org.gradle.api.Project; -import org.gradle.api.internal.tasks.testing.report.HtmlTestReport; -import org.gradle.api.internal.tasks.testing.report.TestReporter; +import org.gradle.api.flow.FlowAction; +import org.gradle.api.flow.FlowParameters; +import org.gradle.api.flow.FlowScope; +import org.gradle.api.provider.Property; +import org.gradle.api.tasks.Input; import org.gradle.api.tasks.testing.AbstractTestTask; -import org.gradle.internal.operations.BuildOperationExecutor; -import org.gradle.internal.operations.BuildOperationRunner; -import org.gradle.util.GradleVersion; -// CHECKSTYLE:ON /** - * In its current form, this plugin may generously be described as "a workaround". - * I've filed gradle#17966 - * upstream to find a better solution. - * We may be able to consume the xml test report and generate our own html based on that - * if the current approach becomes troublesome, that would allow us to color individual - * lines much like our intellij plugin. + * Plugin that formats witchcraft structured logging output in HTML test reports. + * Post-processes generated HTML files to replace JSON log lines with formatted versions. */ public abstract class TestReportFormattingPlugin implements Plugin { + @Inject + protected abstract FlowScope getFlowScope(); + @Override - @SuppressWarnings("Slf4jLogsafeArgs") public final void apply(Project project) { - project.getTasks().withType(AbstractTestTask.class).configureEach(task -> { - try { - Method method = AbstractTestTask.class.getDeclaredMethod("setTestReporter", TestReporter.class); - method.setAccessible(true); - - method.invoke(task, new FormattingTestReporter(getFormattingDelegate())); - } catch (ReflectiveOperationException e) { - project.getLogger() - .error( - "Failed to update task '{}' TestReporter to format structured logging output", - task.getName(), - e); - } - }); + project.getGradle().projectsEvaluated(_gradle -> project.getTasks() + .withType(AbstractTestTask.class) + .forEach(task -> Optional.ofNullable(task.getReports() + .getHtml() + .getOutputLocation() + .getAsFile() + .getOrNull()) + .ifPresent(reportDir -> getFlowScope() + .always(FormatReportAction.class, spec -> spec.getParameters() + .getReportDir() + .set(reportDir))))); } - /** - * The internal gradle test report classes changed in 8.11. DefaultTestReport was renamed to HtmlTestReport and - * also stopped extending the TestReporter interface. So the delegate for the formatting reporter varies either - * an HtmlTestReport to be compatible with gradle 8.11+ or DefaultTestReport for lower versions. - */ - private Object getFormattingDelegate() { - boolean greaterThan8Point11 = GradleVersion.current().compareTo(GradleVersion.version("8.11")) >= 0; - if (greaterThan8Point11) { - return new HtmlTestReport(getBuildOperationRunner(), getBuildOperationExecutor()); - } else { - return createDefaultTestReport(); + public static final class FormatReportAction implements FlowAction { + interface Parameters extends FlowParameters { + @Input + Property getReportDir(); } - } - /** - * The constructor for DefaultTestReport changed in gradle 8.8. Dynamically invoke based on runtime version. - */ - private TestReporter createDefaultTestReport() { - boolean greaterThan8Point8 = GradleVersion.current().compareTo(GradleVersion.version("8.8")) >= 0; - - try { - Class defaultTestReporterClass = (Class) - Class.forName("org.gradle.api.internal.tasks.testing.report.DefaultTestReport"); - if (greaterThan8Point8) { - return defaultTestReporterClass - .getDeclaredConstructor(BuildOperationRunner.class, BuildOperationExecutor.class) - .newInstance(getBuildOperationRunner(), getBuildOperationExecutor()); - } else { - return defaultTestReporterClass - .getDeclaredConstructor(BuildOperationExecutor.class) - .newInstance(getBuildOperationExecutor()); - } - } catch (InstantiationException - | IllegalAccessException - | InvocationTargetException - | ClassNotFoundException - | NoSuchMethodException e) { - throw new RuntimeException(e); + @Override + public void execute(Parameters parameters) { + Optional.ofNullable(parameters.getReportDir().getOrNull()) + .filter(File::exists) + .ifPresent(reportDir -> new HtmlReportPostProcessor().processReportDirectory(reportDir)); } } - - @Inject - @SuppressWarnings("DesignForExtension") - protected abstract BuildOperationExecutor getBuildOperationExecutor(); - - @Inject - @SuppressWarnings("DesignForExtension") - protected abstract BuildOperationRunner getBuildOperationRunner(); } diff --git a/gradle-witchcraft-logging/src/test/java/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPluginIntegrationTest.java b/gradle-witchcraft-logging/src/test/java/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPluginIntegrationTest.java index 1facdf1d..cd63f0b6 100644 --- a/gradle-witchcraft-logging/src/test/java/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPluginIntegrationTest.java +++ b/gradle-witchcraft-logging/src/test/java/com/palantir/witchcraft/java/logging/gradle/testreport/TestReportFormattingPluginIntegrationTest.java @@ -20,6 +20,7 @@ import com.palantir.gradle.testing.execution.GradleInvoker; import com.palantir.gradle.testing.execution.InvocationResult; +import com.palantir.gradle.testing.files.arbitrary.ArbitraryFile; import com.palantir.gradle.testing.junit.GradlePluginTests; import com.palantir.gradle.testing.project.RootProject; import org.junit.jupiter.api.Test; @@ -88,7 +89,9 @@ void formats_test_report_stdout_and_stderr(GradleInvoker gradle, RootProject roo testImplementation 'junit:junit:4.13.2' } - sourceCompatibility = 11 + java { + sourceCompatibility = 11 + } """); rootProject.testSourceSet().java().writeClass(SIMPLE_TEST_CLASS); @@ -99,9 +102,16 @@ void formats_test_report_stdout_and_stderr(GradleInvoker gradle, RootProject roo assertThat(testResult).task(":compileTestJava").upToDate(); - rootProject - .buildDir() - .file("reports/tests/test/classes/com.palantir.SimpleTest.html") + // Gradle 8 uses classes/com.palantir.SimpleTest.html + // Gradle 9 uses com.palantir.SimpleTest/simpleTest.html + ArbitraryFile legacyReportFile = + rootProject.buildDir().file("reports/tests/test/classes/com.palantir.SimpleTest.html"); + ArbitraryFile newReportFile = + rootProject.buildDir().file("reports/tests/test/com.palantir.SimpleTest/simpleTest.html"); + + ArbitraryFile reportFile = legacyReportFile.path().toFile().exists() ? legacyReportFile : newReportFile; + + reportFile .assertThat() .content() .contains("==Service==") diff --git a/gradle/gradle-test-versions.yml b/gradle/gradle-test-versions.yml index 6fe9aa9b..21e8fe68 100644 --- a/gradle/gradle-test-versions.yml +++ b/gradle/gradle-test-versions.yml @@ -1,5 +1,5 @@ major-versions: + 9: 9.3.0 8: 8.14.3 - 7: 7.6.4 extra-versions: - 8.8