Skip to content
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
import com.palantir.witchcraft.java.logging.format.LogParser;
import java.io.File;
import java.io.IOException;
import java.io.StringWriter;
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;
Expand Down Expand Up @@ -61,6 +61,21 @@ private static final class FormattingTestResultsProvider implements TestResultsP
writer.write("\n");
Comment thread
ash211 marked this conversation as resolved.
Outdated
}
: Writable.NOP));
private static final BiConsumer<String, Writer> LINE_PROCESSOR = (line, outputWriter) -> {
try {
PARSER.tryParse(line)
.orElseGet(() -> out -> {
try {
out.write(line);
} catch (IOException e) {
throw new RuntimeException(e);
Comment thread
ash211 marked this conversation as resolved.
Outdated
}
})
.write(outputWriter);
} catch (IOException e) {
throw new RuntimeException(e);
Comment thread
ash211 marked this conversation as resolved.
Outdated
}
};

private final TestResultsProvider delegate;

Expand All @@ -69,30 +84,58 @@ private static final class FormattingTestResultsProvider implements TestResultsP
}

@Override
@SuppressWarnings("StringSplitter")
public void writeAllOutput(long classId, TestOutputEvent.Destination destination, Writer writer) {
if (destination == TestOutputEvent.Destination.StdErr
|| destination == TestOutputEvent.Destination.StdOut) {
StringWriter stringWriter = new StringWriter();
delegate.writeAllOutput(classId, destination, stringWriter);
String contents = stringWriter.toString();
for (String line : contents.split("\n")) {
try {
PARSER.tryParse(line)
.orElseGet(() -> out -> {
out.write(line);
out.write("\n");
})
.write(writer);
} catch (IOException e) {
throw new RuntimeException(e);
}
}
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<String, Writer> lineProcessor;
private final StringBuilder lineBuffer = new StringBuilder();

LineProcessingWriter(Writer delegate, BiConsumer<String, Writer> 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");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to synchronize on the writer to ensure this is written atomically?

Suggested change
lineProcessor.accept(line, delegate);
delegate.write("\n");
synchronized (this) {
lineProcessor.accept(line, delegate);
delegate.write('\n');
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're going to make this LineProcessingWriter thread-safe, then I think we need to protect the lineBuffer from concurrent writes, and maybe then add synchronized keyword to the write() and close() methods.

I'm not sure we need it to be thread-safe though... even multiple calls to the writeAllOutput method here would create new LineProcessingWriters

Copy link
Copy Markdown

@schlosna schlosna Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java.io.Writer exposes a protected lock field for synchronization to ensure that output (e.g. line with new line) is written as an atomic unit. I think something like #763 would help here in cases where there might be concurrent things writing to stdout/stderr. I think we can also provide an optimized write(String str, int off, int len) that uses vectorized String#indexOf and bulk char array copy from String to StringBuilder.

lineBuffer.setLength(0);
}
}

@Override
public void writeNonTestOutput(long classId, TestOutputEvent.Destination destination, Writer writer) {
delegate.writeNonTestOutput(classId, destination, writer);
Expand Down