Github user jaikiran commented on a diff in the pull request: https://github.com/apache/ant/pull/60#discussion_r172044665 --- Diff: src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java --- @@ -0,0 +1,295 @@ +package org.apache.tools.ant.taskdefs.optional.junitlauncher; + +import org.apache.tools.ant.Project; +import org.apache.tools.ant.Task; +import org.apache.tools.ant.util.FileUtils; +import org.junit.platform.engine.TestSource; +import org.junit.platform.engine.support.descriptor.ClassSource; +import org.junit.platform.launcher.TestIdentifier; +import org.junit.platform.launcher.TestPlan; + +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.Closeable; +import java.io.FileOutputStream; +import java.io.FileReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.Reader; +import java.io.Writer; +import java.nio.BufferOverflowException; +import java.nio.ByteBuffer; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Objects; +import java.util.Optional; + +/** + * Contains some common behaviour that's used by our internal {@link TestResultFormatter}s + */ +abstract class AbstractJUnitResultFormatter implements TestResultFormatter { + + + protected static String NEW_LINE = System.getProperty("line.separator"); + protected Task task; + + private SysOutErrContentStore sysOutStore; + private SysOutErrContentStore sysErrStore; + + @Override + public void sysOutAvailable(final byte[] data) { + if (this.sysOutStore == null) { + this.sysOutStore = new SysOutErrContentStore(true); + } + try { + this.sysOutStore.store(data); + } catch (IOException e) { + handleException(e); + return; + } + } + + @Override + public void sysErrAvailable(final byte[] data) { + if (this.sysErrStore == null) { + this.sysErrStore = new SysOutErrContentStore(false); + } + try { + this.sysErrStore.store(data); + } catch (IOException e) { + handleException(e); + return; + } + } + + @Override + public void setExecutingTask(final Task task) { + this.task = task; + } + + /** + * @return Returns true if there's any stdout data, that was generated during the + * tests, is available for use. Else returns false. + */ + boolean hasSysOut() { + return this.sysOutStore != null && this.sysOutStore.hasData(); + } + + /** + * @return Returns true if there's any stderr data, that was generated during the + * tests, is available for use. Else returns false. + */ + boolean hasSysErr() { + return this.sysErrStore != null && this.sysErrStore.hasData(); + } + + /** + * @return Returns a {@link Reader} for reading any stdout data that was generated + * during the test execution. It is expected that the {@link #hasSysOut()} be first + * called to see if any such data is available and only if there is, then this method + * be called + * @throws IOException If there's any I/O problem while creating the {@link Reader} + */ + Reader getSysOutReader() throws IOException { + return this.sysOutStore.getReader(); + } + + /** + * @return Returns a {@link Reader} for reading any stderr data that was generated + * during the test execution. It is expected that the {@link #hasSysErr()} be first + * called to see if any such data is available and only if there is, then this method + * be called + * @throws IOException If there's any I/O problem while creating the {@link Reader} + */ + Reader getSysErrReader() throws IOException { + return this.sysErrStore.getReader(); + } + + /** + * Writes out any stdout data that was generated during the + * test execution. If there was no such data then this method just returns. + * + * @param writer The {@link Writer} to use. Cannot be null. + * @throws IOException If any I/O problem occurs during writing the data + */ + void writeSysOut(final Writer writer) throws IOException { + Objects.requireNonNull(writer, "Writer cannot be null"); + this.writeFrom(this.sysOutStore, writer); + } + + /** + * Writes out any stderr data that was generated during the + * test execution. If there was no such data then this method just returns. + * + * @param writer The {@link Writer} to use. Cannot be null. + * @throws IOException If any I/O problem occurs during writing the data + */ + void writeSysErr(final Writer writer) throws IOException { + Objects.requireNonNull(writer, "Writer cannot be null"); + this.writeFrom(this.sysErrStore, writer); + } + + static Optional<TestIdentifier> traverseAndFindTestClass(final TestPlan testPlan, final TestIdentifier testIdentifier) { + if (isTestClass(testIdentifier).isPresent()) { + return Optional.of(testIdentifier); + } + final Optional<TestIdentifier> parent = testPlan.getParent(testIdentifier); + return parent.isPresent() ? traverseAndFindTestClass(testPlan, parent.get()) : Optional.empty(); + } + + static Optional<ClassSource> isTestClass(final TestIdentifier testIdentifier) { + if (testIdentifier == null) { + return Optional.empty(); + } + final Optional<TestSource> source = testIdentifier.getSource(); + if (!source.isPresent()) { + return Optional.empty(); + } + final TestSource testSource = source.get(); + if (testSource instanceof ClassSource) { + return Optional.of((ClassSource) testSource); + } + return Optional.empty(); + } + + private void writeFrom(final SysOutErrContentStore store, final Writer writer) throws IOException { + final char[] chars = new char[1024]; + int numRead = -1; + try (final Reader reader = store.getReader()) { + while ((numRead = reader.read(chars)) != -1) { + writer.write(chars, 0, numRead); + } + } + } + + @Override + public void close() throws IOException { + FileUtils.close(this.sysOutStore); + FileUtils.close(this.sysErrStore); + } + + protected void handleException(final Throwable t) { + // we currently just log it and move on. + task.getProject().log("Exception in listener " + this.getClass().getName(), t, Project.MSG_DEBUG); + } + + + /* + A "store" for sysout/syserr content that gets sent to the AbstractJUnitResultFormatter. + This store first uses a relatively decent sized in-memory buffer for storing the sysout/syserr + content. This in-memory buffer will be used as long as it can fit in the new content that + keeps coming in. When the size limit is reached, this store switches to a file based store + by creating a temporarily file and writing out the already in-memory held buffer content + and any new content that keeps arriving to this store. Once the file has been created, + the in-memory buffer will never be used any more and in fact is destroyed as soon as the + file is created. + Instances of this class are not thread-safe and users of this class are expected to use necessary thread + safety guarantees, if they want to use an instance of this class by multiple threads. + */ + private static final class SysOutErrContentStore implements Closeable { + private static final int DEFAULT_CAPACITY_IN_BYTES = 50 * 1024; // 50 KB + private static final Reader EMPTY_READER = new Reader() { + @Override + public int read(final char[] cbuf, final int off, final int len) throws IOException { + return -1; + } + + @Override + public void close() throws IOException { + } + }; + + private final String tmpFileSuffix; + private ByteBuffer inMemoryStore = ByteBuffer.allocate(DEFAULT_CAPACITY_IN_BYTES); + private boolean usingFileStore = false; + private Path filePath; + private FileOutputStream fileOutputStream; + + private SysOutErrContentStore(final boolean isSysOut) { + this.tmpFileSuffix = isSysOut ? ".sysout" : ".syserr"; + } + + private void store(final byte[] data) throws IOException { + if (this.usingFileStore) { + this.storeToFile(data, 0, data.length); + return; + } + // we haven't yet created a file store and the data can fit in memory, + // so we write it in our buffer + try { + this.inMemoryStore.put(data); + return; + } catch (BufferOverflowException boe) { + // the buffer capacity can't hold this incoming data, so this + // incoming data hasn't been transferred to the buffer. let's + // now fall back to a file store + this.usingFileStore = true; --- End diff -- >> Also, is there any chance this method could be invoked concurrently? I haven't fully checked every execution path so maybe you are synchronizing somewhere. Tests could spawn new threads you are not aware of and these threads could be writing to syserr/out concurrently. One thing I didn't note in the implementation comments is that the `sysOutAvailable` and `sysErrAvailable` on these result formatter implementations will always be invoked by a single thread - the thread that we create and control `SysOutErrContentDeliverer`. Multiple threads write out to our redirected sysout/syserr stream which then gets read by the `SysOutErrStreamReader` thread which then hands off that read data to the `SysOutErrContentDeliverer` thread to have it delivered to these result formatters.
--- --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org