ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bodewig <...@git.apache.org>
Subject [GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task
Date Sun, 04 Mar 2018 10:23:50 GMT
Github user bodewig commented on a diff in the pull request:

    https://github.com/apache/ant/pull/60#discussion_r172043778
  
    --- 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?
    
    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.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message