[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

classic Classic list List threaded Threaded
51 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

bodewig
GitHub user jaikiran opened a pull request:

    https://github.com/apache/ant/pull/60

    JUnit 5 support - A new junitlauncher task

    This is the initial working version of a new `junitlauncher` task that support using JUnit 5 framework for testing, within Ant build files. The commit in this PR is the initial set of goals that I had in mind for the first release of this task.
   
    The manual for this task can be (temporarily) found at https://builds.apache.org/job/Ant-Build-Jaikiran/ws/manual/Tasks/junitlauncher.html

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jaikiran/ant junit5

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/ant/pull/60.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #60
   
----
commit 991bddff07b4fed1db2dc7f6b83f47557e62213b
Author: Jaikiran Pai <jaikiran@...>
Date:   2017-12-13T13:37:41Z

    JUnit 5 support - A new junitlauncher task

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
Github user asfgit commented on the issue:

    https://github.com/apache/ant/pull/60
 
   
    Refer to this link for build results (access rights to CI server needed):
    https://builds.apache.org/job/Ant%20Github-PR-Windows/40/



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user asfgit commented on the issue:

    https://github.com/apache/ant/pull/60
 
   
    Refer to this link for build results (access rights to CI server needed):
    https://builds.apache.org/job/Ant%20Github-PR-Linux/34/



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user jaikiran commented on the issue:

    https://github.com/apache/ant/pull/60
 
    Please ignore the Jenkins failures on this PR. I'll sort out that part in a separate discussion.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user jaikiran commented on the issue:

    https://github.com/apache/ant/pull/60
 
    retest this please


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user asfgit commented on the issue:

    https://github.com/apache/ant/pull/60
 
   
    Refer to this link for build results (access rights to CI server needed):
    https://builds.apache.org/job/Ant%20Github-PR-Linux/35/



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user asfgit commented on the issue:

    https://github.com/apache/ant/pull/60
 
   
    Refer to this link for build results (access rights to CI server needed):
    https://builds.apache.org/job/Ant%20Github-PR-Windows/41/



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user asfgit commented on the issue:

    https://github.com/apache/ant/pull/60
 
   
    Refer to this link for build results (access rights to CI server needed):
    https://builds.apache.org/job/Ant%20Github-PR-Linux/38/



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user asfgit commented on the issue:

    https://github.com/apache/ant/pull/60
 
   
    Refer to this link for build results (access rights to CI server needed):
    https://builds.apache.org/job/Ant%20Github-PR-Windows/44/



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user bodewig commented on a diff in the pull request:

    https://github.com/apache/ant/pull/60#discussion_r168963955
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java ---
    @@ -0,0 +1,139 @@
    +package org.apache.tools.ant.taskdefs.optional.junitlauncher;
    +
    +import org.apache.tools.ant.Project;
    +import org.apache.tools.ant.Task;
    +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.IOException;
    +import java.io.InputStream;
    +import java.io.OutputStream;
    +import java.io.Writer;
    +import java.nio.file.Files;
    +import java.nio.file.Path;
    +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 Path sysOutFilePath;
    +    protected Path sysErrFilePath;
    +    protected Task task;
    +
    +    private OutputStream sysOutStream;
    +    private OutputStream sysErrStream;
    +
    +    @Override
    +    public void sysOutAvailable(final byte[] data) {
    +        if (this.sysOutStream == null) {
    +            try {
    +                this.sysOutFilePath = Files.createTempFile(null, "sysout");
    +                this.sysOutFilePath.toFile().deleteOnExit();
    +                this.sysOutStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysOutStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void sysErrAvailable(final byte[] data) {
    +        if (this.sysErrStream == null) {
    +            try {
    +                this.sysErrFilePath = Files.createTempFile(null, "syserr");
    +                this.sysErrFilePath.toFile().deleteOnExit();
    +                this.sysErrStream = Files.newOutputStream(this.sysOutFilePath);
    --- End diff --
   
    copy-paste error, you want that to be `sysErrFilePath` :-)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user bodewig commented on a diff in the pull request:

    https://github.com/apache/ant/pull/60#discussion_r168964295
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java ---
    @@ -0,0 +1,139 @@
    +package org.apache.tools.ant.taskdefs.optional.junitlauncher;
    +
    +import org.apache.tools.ant.Project;
    +import org.apache.tools.ant.Task;
    +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.IOException;
    +import java.io.InputStream;
    +import java.io.OutputStream;
    +import java.io.Writer;
    +import java.nio.file.Files;
    +import java.nio.file.Path;
    +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 Path sysOutFilePath;
    +    protected Path sysErrFilePath;
    +    protected Task task;
    +
    +    private OutputStream sysOutStream;
    +    private OutputStream sysErrStream;
    +
    +    @Override
    +    public void sysOutAvailable(final byte[] data) {
    +        if (this.sysOutStream == null) {
    +            try {
    +                this.sysOutFilePath = Files.createTempFile(null, "sysout");
    +                this.sysOutFilePath.toFile().deleteOnExit();
    +                this.sysOutStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysOutStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void sysErrAvailable(final byte[] data) {
    +        if (this.sysErrStream == null) {
    +            try {
    +                this.sysErrFilePath = Files.createTempFile(null, "syserr");
    +                this.sysErrFilePath.toFile().deleteOnExit();
    +                this.sysErrStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysErrStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void setExecutingTask(final Task task) {
    +        this.task = task;
    +    }
    +
    +    protected void writeSysOut(final Writer writer) throws IOException {
    +        this.writeFrom(this.sysOutFilePath, writer);
    +    }
    +
    +    protected void writeSysErr(final Writer writer) throws IOException {
    +        this.writeFrom(this.sysErrFilePath, 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 Path path, final Writer writer) throws IOException {
    +        final byte[] content = new byte[1024];
    +        int numBytes;
    +        try (final InputStream is = Files.newInputStream(path)) {
    +            while ((numBytes = is.read(content)) != -1) {
    +                writer.write(new String(content, 0, numBytes));
    +            }
    --- End diff --
   
    I don't think this is going to work reliably. The `read` may have split a multi-byte sequence at the end of `content` and then creating a string from it is going to break. Is there any reason you want to use a stream when reading the temporary file rather than a reader?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user bodewig commented on a diff in the pull request:

    https://github.com/apache/ant/pull/60#discussion_r168964381
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java ---
    @@ -0,0 +1,139 @@
    +package org.apache.tools.ant.taskdefs.optional.junitlauncher;
    +
    +import org.apache.tools.ant.Project;
    +import org.apache.tools.ant.Task;
    +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.IOException;
    +import java.io.InputStream;
    +import java.io.OutputStream;
    +import java.io.Writer;
    +import java.nio.file.Files;
    +import java.nio.file.Path;
    +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 Path sysOutFilePath;
    +    protected Path sysErrFilePath;
    +    protected Task task;
    +
    +    private OutputStream sysOutStream;
    +    private OutputStream sysErrStream;
    +
    +    @Override
    +    public void sysOutAvailable(final byte[] data) {
    +        if (this.sysOutStream == null) {
    +            try {
    +                this.sysOutFilePath = Files.createTempFile(null, "sysout");
    +                this.sysOutFilePath.toFile().deleteOnExit();
    +                this.sysOutStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysOutStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void sysErrAvailable(final byte[] data) {
    +        if (this.sysErrStream == null) {
    +            try {
    +                this.sysErrFilePath = Files.createTempFile(null, "syserr");
    +                this.sysErrFilePath.toFile().deleteOnExit();
    +                this.sysErrStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysErrStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void setExecutingTask(final Task task) {
    +        this.task = task;
    +    }
    +
    +    protected void writeSysOut(final Writer writer) throws IOException {
    +        this.writeFrom(this.sysOutFilePath, writer);
    +    }
    +
    +    protected void writeSysErr(final Writer writer) throws IOException {
    +        this.writeFrom(this.sysErrFilePath, 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 Path path, final Writer writer) throws IOException {
    +        final byte[] content = new byte[1024];
    +        int numBytes;
    +        try (final InputStream is = Files.newInputStream(path)) {
    +            while ((numBytes = is.read(content)) != -1) {
    +                writer.write(new String(content, 0, numBytes));
    +            }
    +        }
    +    }
    +
    +    @Override
    +    public void close() throws IOException {
    +        if (this.sysOutStream != null) {
    +            try {
    +                this.sysOutStream.close();
    +            } catch (Exception e) {
    +                // ignore
    +            }
    +        }
    --- End diff --
   
    `FileUtils.close`?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user jaikiran commented on a diff in the pull request:

    https://github.com/apache/ant/pull/60#discussion_r168983118
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java ---
    @@ -0,0 +1,139 @@
    +package org.apache.tools.ant.taskdefs.optional.junitlauncher;
    +
    +import org.apache.tools.ant.Project;
    +import org.apache.tools.ant.Task;
    +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.IOException;
    +import java.io.InputStream;
    +import java.io.OutputStream;
    +import java.io.Writer;
    +import java.nio.file.Files;
    +import java.nio.file.Path;
    +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 Path sysOutFilePath;
    +    protected Path sysErrFilePath;
    +    protected Task task;
    +
    +    private OutputStream sysOutStream;
    +    private OutputStream sysErrStream;
    +
    +    @Override
    +    public void sysOutAvailable(final byte[] data) {
    +        if (this.sysOutStream == null) {
    +            try {
    +                this.sysOutFilePath = Files.createTempFile(null, "sysout");
    +                this.sysOutFilePath.toFile().deleteOnExit();
    +                this.sysOutStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysOutStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void sysErrAvailable(final byte[] data) {
    +        if (this.sysErrStream == null) {
    +            try {
    +                this.sysErrFilePath = Files.createTempFile(null, "syserr");
    +                this.sysErrFilePath.toFile().deleteOnExit();
    +                this.sysErrStream = Files.newOutputStream(this.sysOutFilePath);
    --- End diff --
   
    Indeed :) Fixed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user jaikiran commented on a diff in the pull request:

    https://github.com/apache/ant/pull/60#discussion_r168983221
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java ---
    @@ -0,0 +1,139 @@
    +package org.apache.tools.ant.taskdefs.optional.junitlauncher;
    +
    +import org.apache.tools.ant.Project;
    +import org.apache.tools.ant.Task;
    +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.IOException;
    +import java.io.InputStream;
    +import java.io.OutputStream;
    +import java.io.Writer;
    +import java.nio.file.Files;
    +import java.nio.file.Path;
    +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 Path sysOutFilePath;
    +    protected Path sysErrFilePath;
    +    protected Task task;
    +
    +    private OutputStream sysOutStream;
    +    private OutputStream sysErrStream;
    +
    +    @Override
    +    public void sysOutAvailable(final byte[] data) {
    +        if (this.sysOutStream == null) {
    +            try {
    +                this.sysOutFilePath = Files.createTempFile(null, "sysout");
    +                this.sysOutFilePath.toFile().deleteOnExit();
    +                this.sysOutStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysOutStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void sysErrAvailable(final byte[] data) {
    +        if (this.sysErrStream == null) {
    +            try {
    +                this.sysErrFilePath = Files.createTempFile(null, "syserr");
    +                this.sysErrFilePath.toFile().deleteOnExit();
    +                this.sysErrStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysErrStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void setExecutingTask(final Task task) {
    +        this.task = task;
    +    }
    +
    +    protected void writeSysOut(final Writer writer) throws IOException {
    +        this.writeFrom(this.sysOutFilePath, writer);
    +    }
    +
    +    protected void writeSysErr(final Writer writer) throws IOException {
    +        this.writeFrom(this.sysErrFilePath, 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 Path path, final Writer writer) throws IOException {
    +        final byte[] content = new byte[1024];
    +        int numBytes;
    +        try (final InputStream is = Files.newInputStream(path)) {
    +            while ((numBytes = is.read(content)) != -1) {
    +                writer.write(new String(content, 0, numBytes));
    +            }
    --- End diff --
   
    You are right, good catch! I hadn't considered that scenario. I have now switched to using a reader instead.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user jaikiran commented on a diff in the pull request:

    https://github.com/apache/ant/pull/60#discussion_r168983237
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java ---
    @@ -0,0 +1,139 @@
    +package org.apache.tools.ant.taskdefs.optional.junitlauncher;
    +
    +import org.apache.tools.ant.Project;
    +import org.apache.tools.ant.Task;
    +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.IOException;
    +import java.io.InputStream;
    +import java.io.OutputStream;
    +import java.io.Writer;
    +import java.nio.file.Files;
    +import java.nio.file.Path;
    +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 Path sysOutFilePath;
    +    protected Path sysErrFilePath;
    +    protected Task task;
    +
    +    private OutputStream sysOutStream;
    +    private OutputStream sysErrStream;
    +
    +    @Override
    +    public void sysOutAvailable(final byte[] data) {
    +        if (this.sysOutStream == null) {
    +            try {
    +                this.sysOutFilePath = Files.createTempFile(null, "sysout");
    +                this.sysOutFilePath.toFile().deleteOnExit();
    +                this.sysOutStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysOutStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void sysErrAvailable(final byte[] data) {
    +        if (this.sysErrStream == null) {
    +            try {
    +                this.sysErrFilePath = Files.createTempFile(null, "syserr");
    +                this.sysErrFilePath.toFile().deleteOnExit();
    +                this.sysErrStream = Files.newOutputStream(this.sysOutFilePath);
    +            } catch (IOException e) {
    +                handleException(e);
    +                return;
    +            }
    +        }
    +        try {
    +            this.sysErrStream.write(data);
    +        } catch (IOException e) {
    +            handleException(e);
    +            return;
    +        }
    +    }
    +
    +    @Override
    +    public void setExecutingTask(final Task task) {
    +        this.task = task;
    +    }
    +
    +    protected void writeSysOut(final Writer writer) throws IOException {
    +        this.writeFrom(this.sysOutFilePath, writer);
    +    }
    +
    +    protected void writeSysErr(final Writer writer) throws IOException {
    +        this.writeFrom(this.sysErrFilePath, 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 Path path, final Writer writer) throws IOException {
    +        final byte[] content = new byte[1024];
    +        int numBytes;
    +        try (final InputStream is = Files.newInputStream(path)) {
    +            while ((numBytes = is.read(content)) != -1) {
    +                writer.write(new String(content, 0, numBytes));
    +            }
    +        }
    +    }
    +
    +    @Override
    +    public void close() throws IOException {
    +        if (this.sysOutStream != null) {
    +            try {
    +                this.sysOutStream.close();
    +            } catch (Exception e) {
    +                // ignore
    +            }
    +        }
    --- End diff --
   
    Yes, make sense. Fixed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user jaikiran commented on the issue:

    https://github.com/apache/ant/pull/60
 
    @bodewig, Thank you very much for the review. Based on the review comments, I have updated the PR with additional separate commits (for easier reference). I'll be squashing all these commits together during merge.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user asfgit commented on the issue:

    https://github.com/apache/ant/pull/60
 
   
    Refer to this link for build results (access rights to CI server needed):
    https://builds.apache.org/job/Ant%20Github-PR-Windows/45/



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user asfgit commented on the issue:

    https://github.com/apache/ant/pull/60
 
   
    Refer to this link for build results (access rights to CI server needed):
    https://builds.apache.org/job/Ant%20Github-PR-Linux/39/



---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user bodewig commented on a diff in the pull request:

    https://github.com/apache/ant/pull/60#discussion_r169243988
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java ---
    @@ -0,0 +1,508 @@
    +package org.apache.tools.ant.taskdefs.optional.junitlauncher;
    +
    +import org.apache.tools.ant.AntClassLoader;
    +import org.apache.tools.ant.BuildException;
    +import org.apache.tools.ant.Project;
    +import org.apache.tools.ant.Task;
    +import org.apache.tools.ant.types.Path;
    +import org.apache.tools.ant.util.KeepAliveOutputStream;
    +import org.junit.platform.launcher.Launcher;
    +import org.junit.platform.launcher.LauncherDiscoveryRequest;
    +import org.junit.platform.launcher.TestExecutionListener;
    +import org.junit.platform.launcher.TestPlan;
    +import org.junit.platform.launcher.core.LauncherFactory;
    +import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
    +import org.junit.platform.launcher.listeners.TestExecutionSummary;
    +
    +import java.io.Closeable;
    +import java.io.IOException;
    +import java.io.InputStream;
    +import java.io.OutputStream;
    +import java.io.PipedInputStream;
    +import java.io.PipedOutputStream;
    +import java.io.PrintStream;
    +import java.nio.file.Files;
    +import java.nio.file.Paths;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.Collections;
    +import java.util.List;
    +import java.util.Optional;
    +import java.util.concurrent.BlockingQueue;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.LinkedBlockingQueue;
    +import java.util.concurrent.TimeUnit;
    +
    +/**
    + * An Ant {@link Task} responsible for launching the JUnit platform for running tests.
    + * This requires a minimum of JUnit 5, since that's the version in which the JUnit platform launcher
    + * APIs were introduced.
    + * <p>
    + * This task in itself doesn't run the JUnit tests, instead the sole responsibility of
    + * this task is to setup the JUnit platform launcher, build requests, launch those requests and then parse the
    + * result of the execution to present in a way that's been configured on this Ant task.
    + * </p>
    + * <p>
    + * Furthermore, this task allows users control over which classes to select for passing on to the JUnit 5
    + * platform for test execution. It however, is solely the JUnit 5 platform, backed by test engines that
    + * decide and execute the tests.
    + *
    + * @see <a href="https://junit.org/junit5/">JUnit 5 documentation</a> for more details
    + * on how JUnit manages the platform and the test engines.
    + */
    +public class JUnitLauncherTask extends Task {
    +
    +    private Path classPath;
    +    private boolean haltOnFailure;
    +    private String failureProperty;
    +    private final List<TestDefinition> tests = new ArrayList<>();
    +    private final List<ListenerDefinition> listeners = new ArrayList<>();
    +
    +    public JUnitLauncherTask() {
    +    }
    +
    +    @Override
    +    public void execute() throws BuildException {
    +        final ClassLoader previousClassLoader = Thread.currentThread().getContextClassLoader();
    +        try {
    +            final ClassLoader executionCL = createClassLoaderForTestExecution();
    +            Thread.currentThread().setContextClassLoader(executionCL);
    +            final Launcher launcher = LauncherFactory.create();
    +            final List<TestRequest> requests = buildTestRequests();
    +            for (final TestRequest testRequest : requests) {
    +                try {
    +                    final TestDefinition test = testRequest.getOwner();
    +                    final LauncherDiscoveryRequest request = testRequest.getDiscoveryRequest().build();
    +                    final List<TestExecutionListener> testExecutionListeners = new ArrayList<>();
    +                    // a listener that we always put at the front of list of listeners
    +                    // for this request.
    +                    final Listener firstListener = new Listener();
    +                    // we always enroll the summary generating listener, to the request, so that we
    +                    // get to use some of the details of the summary for our further decision making
    +                    testExecutionListeners.add(firstListener);
    +                    testExecutionListeners.addAll(getListeners(testRequest, executionCL));
    +                    final PrintStream originalSysOut = System.out;
    +                    final PrintStream originalSysErr = System.err;
    +                    try {
    +                        firstListener.switchedSysOutHandle = trySwitchSysOut(testRequest);
    +                        firstListener.switchedSysErrHandle = trySwitchSysErr(testRequest);
    +                        launcher.execute(request, testExecutionListeners.toArray(new TestExecutionListener[testExecutionListeners.size()]));
    +                    } finally {
    +                        // switch back sysout/syserr to the original
    +                        try {
    +                            System.setOut(originalSysOut);
    +                        } catch (Exception e) {
    +                            // ignore
    +                        }
    +                        try {
    +                            System.setErr(originalSysErr);
    +                        } catch (Exception e) {
    +                            // ignore
    +                        }
    +                    }
    +                    handleTestExecutionCompletion(test, firstListener.getSummary());
    +                } finally {
    +                    try {
    +                        testRequest.close();
    +                    } catch (Exception e) {
    +                        // log and move on
    +                        log("Failed to cleanly close test request", e, Project.MSG_DEBUG);
    +                    }
    +                }
    +            }
    +        } finally {
    +            Thread.currentThread().setContextClassLoader(previousClassLoader);
    +        }
    +    }
    +
    +    /**
    +     * @return Creates and returns the a {@link Path} which will be used as the classpath of this
    +     * task. This classpath will then be used for execution of the tests
    +     */
    +    public Path createClassPath() {
    +        this.classPath = new Path(getProject());
    --- End diff --
   
    if users specify multiple nested classpath elements, only the last one will be used. You may want to return an already created `this.classpath` or move on to an `add` method and collect the paths inside of a wrapper `Path`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

bodewig
In reply to this post by bodewig
Github user bodewig commented on a diff in the pull request:

    https://github.com/apache/ant/pull/60#discussion_r169245116
 
    --- Diff: src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java ---
    @@ -0,0 +1,508 @@
    +package org.apache.tools.ant.taskdefs.optional.junitlauncher;
    +
    +import org.apache.tools.ant.AntClassLoader;
    +import org.apache.tools.ant.BuildException;
    +import org.apache.tools.ant.Project;
    +import org.apache.tools.ant.Task;
    +import org.apache.tools.ant.types.Path;
    +import org.apache.tools.ant.util.KeepAliveOutputStream;
    +import org.junit.platform.launcher.Launcher;
    +import org.junit.platform.launcher.LauncherDiscoveryRequest;
    +import org.junit.platform.launcher.TestExecutionListener;
    +import org.junit.platform.launcher.TestPlan;
    +import org.junit.platform.launcher.core.LauncherFactory;
    +import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
    +import org.junit.platform.launcher.listeners.TestExecutionSummary;
    +
    +import java.io.Closeable;
    +import java.io.IOException;
    +import java.io.InputStream;
    +import java.io.OutputStream;
    +import java.io.PipedInputStream;
    +import java.io.PipedOutputStream;
    +import java.io.PrintStream;
    +import java.nio.file.Files;
    +import java.nio.file.Paths;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.Collections;
    +import java.util.List;
    +import java.util.Optional;
    +import java.util.concurrent.BlockingQueue;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.LinkedBlockingQueue;
    +import java.util.concurrent.TimeUnit;
    +
    +/**
    + * An Ant {@link Task} responsible for launching the JUnit platform for running tests.
    + * This requires a minimum of JUnit 5, since that's the version in which the JUnit platform launcher
    + * APIs were introduced.
    + * <p>
    + * This task in itself doesn't run the JUnit tests, instead the sole responsibility of
    + * this task is to setup the JUnit platform launcher, build requests, launch those requests and then parse the
    + * result of the execution to present in a way that's been configured on this Ant task.
    + * </p>
    + * <p>
    + * Furthermore, this task allows users control over which classes to select for passing on to the JUnit 5
    + * platform for test execution. It however, is solely the JUnit 5 platform, backed by test engines that
    + * decide and execute the tests.
    + *
    + * @see <a href="https://junit.org/junit5/">JUnit 5 documentation</a> for more details
    + * on how JUnit manages the platform and the test engines.
    + */
    +public class JUnitLauncherTask extends Task {
    +
    +    private Path classPath;
    +    private boolean haltOnFailure;
    +    private String failureProperty;
    +    private final List<TestDefinition> tests = new ArrayList<>();
    +    private final List<ListenerDefinition> listeners = new ArrayList<>();
    +
    +    public JUnitLauncherTask() {
    +    }
    +
    +    @Override
    +    public void execute() throws BuildException {
    +        final ClassLoader previousClassLoader = Thread.currentThread().getContextClassLoader();
    +        try {
    +            final ClassLoader executionCL = createClassLoaderForTestExecution();
    +            Thread.currentThread().setContextClassLoader(executionCL);
    +            final Launcher launcher = LauncherFactory.create();
    +            final List<TestRequest> requests = buildTestRequests();
    +            for (final TestRequest testRequest : requests) {
    +                try {
    +                    final TestDefinition test = testRequest.getOwner();
    +                    final LauncherDiscoveryRequest request = testRequest.getDiscoveryRequest().build();
    +                    final List<TestExecutionListener> testExecutionListeners = new ArrayList<>();
    +                    // a listener that we always put at the front of list of listeners
    +                    // for this request.
    +                    final Listener firstListener = new Listener();
    +                    // we always enroll the summary generating listener, to the request, so that we
    +                    // get to use some of the details of the summary for our further decision making
    +                    testExecutionListeners.add(firstListener);
    +                    testExecutionListeners.addAll(getListeners(testRequest, executionCL));
    +                    final PrintStream originalSysOut = System.out;
    +                    final PrintStream originalSysErr = System.err;
    +                    try {
    +                        firstListener.switchedSysOutHandle = trySwitchSysOut(testRequest);
    +                        firstListener.switchedSysErrHandle = trySwitchSysErr(testRequest);
    +                        launcher.execute(request, testExecutionListeners.toArray(new TestExecutionListener[testExecutionListeners.size()]));
    +                    } finally {
    +                        // switch back sysout/syserr to the original
    +                        try {
    +                            System.setOut(originalSysOut);
    +                        } catch (Exception e) {
    +                            // ignore
    +                        }
    +                        try {
    +                            System.setErr(originalSysErr);
    +                        } catch (Exception e) {
    +                            // ignore
    +                        }
    +                    }
    +                    handleTestExecutionCompletion(test, firstListener.getSummary());
    +                } finally {
    +                    try {
    +                        testRequest.close();
    +                    } catch (Exception e) {
    +                        // log and move on
    +                        log("Failed to cleanly close test request", e, Project.MSG_DEBUG);
    +                    }
    +                }
    +            }
    +        } finally {
    +            Thread.currentThread().setContextClassLoader(previousClassLoader);
    +        }
    +    }
    +
    +    /**
    +     * @return Creates and returns the a {@link Path} which will be used as the classpath of this
    +     * task. This classpath will then be used for execution of the tests
    +     */
    +    public Path createClassPath() {
    +        this.classPath = new Path(getProject());
    +        return this.classPath;
    +    }
    +
    +    /**
    +     * @return Creates and returns a {@link SingleTestClass}. This test will be considered part of the
    +     * tests that will be passed on to the underlying JUnit platform for possible execution of the test
    +     */
    +    public SingleTestClass createTest() {
    +        final SingleTestClass test = new SingleTestClass();
    +        this.preConfigure(test);
    --- End diff --
   
    `create*` is called before the attribute setters, If you call `preconfigure` here the test won't see the actual values set at the task level.
   
    `addConfigured` would get called after the attribute setters, alternatively you need to defer pre-configuration until `execute` is called. With both approaches you need to ensure you don't overwrite the test's explicit configuration with values from the task, of course.
   
    Same for `createTestClasses`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

123