From b510f3588d2097beb322e946a944a82c570d2782 Mon Sep 17 00:00:00 2001 From: Jaikiran Pai Date: Fri, 7 Dec 2018 14:00:15 +0530 Subject: [PATCH] [junitlauncher] - Switch back to issuing a separate LauncherDiscoveryRequest for each of the test class that are selected by the usage In 1.10.5 version (the latest released) of Ant, we used to launch multiple LauncherDiscoveryRequest (a JUnit 5 construct), one each for a test class, that belonged to the usage. Commit 3f806148c3f3542b8526d5f91f10d6189bf0607d changed it to launch one single LauncherDiscoveryRequest (and including all those test classes in that request) because it seemed more logical to do so. However, recent tests/experimentation of the master branch has shown that it introduces complexities when the "legacy-xml" listener (the one which supports generating test results in xml format that junitreport understands) is used. These complexities include - expecting each test class to have a separate report file, but isn't limited to that. Solving these issues isn't easy and probably not worth it, given that the only reason we started using a single LauncherDiscoveryRequest is because it just seemed logical and there is no other strong reason to do so. This commit switches back to the behaviour that's been there in 1.10.5 version, to issue multiple separate LauncherDiscoveryRequest(s) one each for the test class that's selected through the use of . --- .../taskdefs/optional/junitlauncher.xml | 6 +- .../junitlauncher/LauncherSupport.java | 55 +++++++++++++------ .../optional/junitlauncher/TestRequest.java | 10 +++- .../confined/ListenerDefinition.java | 21 +------ .../org/example/junitlauncher/Tracker.java | 22 +++++++- 5 files changed, 71 insertions(+), 43 deletions(-) diff --git a/src/etc/testcases/taskdefs/optional/junitlauncher.xml b/src/etc/testcases/taskdefs/optional/junitlauncher.xml index 6dd202d8a..62043690e 100644 --- a/src/etc/testcases/taskdefs/optional/junitlauncher.xml +++ b/src/etc/testcases/taskdefs/optional/junitlauncher.xml @@ -131,12 +131,11 @@ + @@ -216,6 +215,7 @@ + @@ -231,8 +231,6 @@ diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LauncherSupport.java b/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LauncherSupport.java index 194887a16..1155d09fc 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LauncherSupport.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LauncherSupport.java @@ -245,7 +245,23 @@ public class LauncherSupport { private Path getListenerOutputFile(final TestRequest testRequest, final ListenerDefinition listener) { final TestDefinition test = testRequest.getOwner(); - final String filename = listener.requireResultFile(test); + final String filename; + if (listener.getResultFile() != null) { + filename = listener.getResultFile(); + } else { + // compute a file name + final StringBuilder sb = new StringBuilder("TEST-"); + sb.append(testRequest.getName() == null ? "unknown" : testRequest.getName()); + sb.append("."); + final String suffix; + if ("org.apache.tools.ant.taskdefs.optional.junitlauncher.LegacyXmlResultFormatter".equals(listener.getClassName())) { + suffix = "xml"; + } else { + suffix = "txt"; + } + sb.append(suffix); + filename = sb.toString(); + } if (listener.getOutputDir() != null) { // use the output dir defined on the listener return Paths.get(listener.getOutputDir(), filename); @@ -391,19 +407,13 @@ public class LauncherSupport { private List createTestRequests(final TestDefinition test) { - // create a TestRequest and add necessary selectors, filters to it - final LauncherDiscoveryRequestBuilder requestBuilder = LauncherDiscoveryRequestBuilder.request(); - final TestRequest request = new TestRequest(test, requestBuilder); - addDiscoverySelectors(request); - addFilters(request); - return Collections.singletonList(request); - } + // create TestRequest(s) and add necessary selectors, filters to it - private void addDiscoverySelectors(final TestRequest testRequest) { - final TestDefinition test = testRequest.getOwner(); - final LauncherDiscoveryRequestBuilder requestBuilder = testRequest.getDiscoveryRequest(); if (test instanceof SingleTestClass) { final SingleTestClass singleTestClass = (SingleTestClass) test; + final LauncherDiscoveryRequestBuilder requestBuilder = LauncherDiscoveryRequestBuilder.request(); + final TestRequest request = new TestRequest(test, requestBuilder); + request.setName(singleTestClass.getName()); final String[] methods = singleTestClass.getMethods(); if (methods == null) { requestBuilder.selectors(DiscoverySelectors.selectClass(singleTestClass.getName())); @@ -413,19 +423,28 @@ public class LauncherSupport { requestBuilder.selectors(DiscoverySelectors.selectMethod(singleTestClass.getName(), method)); } } - return; + addFilters(request); + return Collections.singletonList(request); } + if (test instanceof TestClasses) { - final TestClasses testClasses = (TestClasses) test; - final List testClassNames = testClasses.getTestClassNames(); - if (testClassNames.isEmpty()) { - return; + final List testClasses = ((TestClasses) test).getTestClassNames(); + if (testClasses.isEmpty()) { + return Collections.emptyList(); } - for (final String testClass : testClassNames) { + final List requests = new ArrayList<>(); + for (final String testClass : testClasses) { + final LauncherDiscoveryRequestBuilder requestBuilder = LauncherDiscoveryRequestBuilder.request(); + final TestRequest request = new TestRequest(test, requestBuilder); + request.setName(testClass); requestBuilder.selectors(DiscoverySelectors.selectClass(testClass)); + addFilters(request); + + requests.add(request); } - return; + return requests; } + return Collections.emptyList(); } /** diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/TestRequest.java b/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/TestRequest.java index ef15536d8..57ad1b4a1 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/TestRequest.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/TestRequest.java @@ -36,7 +36,7 @@ final class TestRequest implements AutoCloseable { private final List closables = new ArrayList<>(); private final List interestedInSysOut = new ArrayList<>(); private final List interestedInSysErr = new ArrayList<>(); - + private String name; TestRequest(final TestDefinition ownerTest, final LauncherDiscoveryRequestBuilder discoveryRequest) { this.ownerTest = ownerTest; @@ -51,6 +51,14 @@ final class TestRequest implements AutoCloseable { return discoveryRequest; } + void setName(final String name) { + this.name = name; + } + + String getName() { + return this.name; + } + void closeUponCompletion(final Closeable closeable) { if (closeable == null) { return; diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/confined/ListenerDefinition.java b/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/confined/ListenerDefinition.java index fea8c79fd..ebb21766e 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/confined/ListenerDefinition.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/confined/ListenerDefinition.java @@ -100,25 +100,8 @@ public class ListenerDefinition { this.resultFile = filename; } - public String requireResultFile(final TestDefinition test) { - if (this.resultFile != null) { - return this.resultFile; - } - final StringBuilder sb = new StringBuilder("TEST-"); - if (test instanceof NamedTest) { - sb.append(((NamedTest) test).getName()); - } else { - sb.append("unknown"); - } - sb.append("."); - final String suffix; - if ("org.apache.tools.ant.taskdefs.optional.junitlauncher.LegacyXmlResultFormatter".equals(this.className)) { - suffix = "xml"; - } else { - suffix = "txt"; - } - sb.append(suffix); - return sb.toString(); + public String getResultFile() { + return this.resultFile; } public void setSendSysOut(final boolean sendSysOut) { diff --git a/src/tests/junit/org/example/junitlauncher/Tracker.java b/src/tests/junit/org/example/junitlauncher/Tracker.java index 28ba1e613..57a3d627e 100644 --- a/src/tests/junit/org/example/junitlauncher/Tracker.java +++ b/src/tests/junit/org/example/junitlauncher/Tracker.java @@ -25,11 +25,15 @@ import org.junit.platform.engine.support.descriptor.ClassSource; import org.junit.platform.engine.support.descriptor.MethodSource; import org.junit.platform.launcher.TestIdentifier; +import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; import java.io.PrintWriter; import java.nio.file.Files; +import java.nio.file.OpenOption; import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardOpenOption; import java.util.List; /** @@ -45,10 +49,23 @@ public class Tracker implements TestResultFormatter { private PrintWriter writer; private TestExecutionContext context; + private OutputStream appendModeFile; @Override public void setDestination(final OutputStream os) { - this.writer = new PrintWriter(os, true); + final String propVal = this.context.getProperties().getProperty("junitlauncher.test.tracker.append.file"); + if (propVal == null) { + this.writer = new PrintWriter(os, true); + return; + } + // ignore the passed outputstream and instead create our own, in append mode + final Path appendModeFilePath = Paths.get(propVal); + try { + this.appendModeFile = Files.newOutputStream(appendModeFilePath, StandardOpenOption.CREATE, StandardOpenOption.APPEND); + this.writer = new PrintWriter(this.appendModeFile, true); + } catch (IOException e) { + throw new RuntimeException(e); + } } @Override @@ -59,6 +76,9 @@ public class Tracker implements TestResultFormatter { @Override public void close() throws IOException { this.writer.flush(); + if (this.appendModeFile != null) { + this.appendModeFile.close(); + } } @Override