Github user jaikiran commented on a diff in the pull request:
https://github.com/apache/ant/pull/60#discussion_r172044509
--- 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 --
>> maybe defer setting the flag until storeToFile below succeeds?
I did actually think of it while coding this part. The reason I decided to
go this way, was to make sure that as soon as the memory size limit is reached,
I prefered it to be better to no more use the in memory store even if the file
store cannot be created/used.
That would imply that the subsequent syserr/sysout message will be lost but
at least that won't lead to the in memory store going past the limit that's set.
The alternate approach would be to let the in-memory store be used even
past its limit (and not lose any sysout/syserr content) if the file store
cannot be created for whatever reason. If that feels more better approach to
take, I can switch this logic in here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org
