Donate current sources from junit-platform-surefire-provider #184

sormuras · 2018-05-03T09:11:02Z

This PR includes enhancements and bug fixes that were applied since the initial code donation.

History of changes: https://github.com/junit-team/junit5/commits/master/junit-platform-surefire-provider

Addresses https://issues.apache.org/jira/browse/SUREFIRE-1330

olamy · 2018-05-03T10:52:00Z

awesome!!

eolivelli

great ! thank you

Tibor17 · 2018-05-03T18:42:02Z

@sormuras
I should perhaps squash all 4 commits in the branch https://gitbox.apache.org/repos/asf?p=maven-surefire.git;a=shortlog;h=refs/heads/1330 and add the last commit 4d9173e .
Do you have permissions in ASF git?

sormuras · 2018-05-03T19:36:40Z

Sure. Go ahead.

I am still waiting for the ASF git permissions.

Tibor17 · 2018-05-03T20:25:35Z

@sormuras
The branch contains all your changes now.

sormuras · 2018-05-04T06:57:43Z

Nice. But animal-sniffer-maven-plugin still fails either by not finding a bunch of methods, like:

surefire-providers\surefire-junit-platform\src\main\java\org\apache\maven\surefire\junitplatform\JUnitPlatformProvider.java:213: Undefined reference: boolean org.junit.platform.commons.util.StringUtils.isNotBlank(String)
surefire-providers\surefire-junit-platform\src\main\java\org\apache\maven\surefire\junitplatform\JUnitPlatformProvider.java:228: Undefined reference: void org.junit.platform.commons.util.Preconditions.condition(boolean, String)

or if I comment the exclude for org.junit.platform:junit-platform-commons out, it seems to choke due to an out-dated ASM version:

org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.codehaus.mojo:animal-sniffer-maven-plugin:1.15:check (signature-check) on project surefire-junit-platform: Execution signature-check of goal org.codehaus.mojo:animal-sniffer-maven-plugin:1.15:check failed.
[...]
Caused by: java.lang.IllegalArgumentException
	at org.objectweb.asm.ClassReader.<init>(Unknown Source)
	at org.objectweb.asm.ClassReader.<init>(Unknown Source)
	at org.objectweb.asm.ClassReader.<init>(Unknown Source)
	at org.codehaus.mojo.animal_sniffer.ClassListBuilder.process(ClassListBuilder.java:71)

How do I fix those build errors?

sormuras · 2018-05-04T07:08:46Z

Working-around it for now by

[...]
<ignores>
    <param>org.junit.platform.commons.*</param>
</ignores>

sormuras · 2018-05-04T20:39:17Z

@Tibor17 Can you please integrate the last two commits as well? The build was successful on my machine and before I write more integration tests, I'd like to know whether I'm on the right track.

sormuras · 2018-05-05T09:43:34Z

Make it three commits. 🔱

Tibor17 · 2018-05-05T10:06:01Z

...it-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java

+/**
+ * JUnitPlatformProvider.
+ *
+ * @since 1.0


Pls put here another version. We can make release of the provider in e.g. 2.22.0.

Fixing all JUnit-related versions to 2.22.0.

Tibor17 · 2018-05-05T10:16:12Z

maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java

@@ -1022,9 +1029,9 @@ private void createDependencyResolver()
        Artifact junitDepArtifact = getJunitDepArtifact();
        return new ProviderList( new DynamicProviderInfo( null ),
                              new TestNgProviderInfo( getTestNgArtifact() ),
+                              new JUnitPlatformProviderInfo( getJunitPlatformArtifact() ),


Here the order of provider-info was changed. Does it mean something important for functionality?

Are the built-in providers documented somewhere? I couldn't find any external information.

The first auto-detected provider wins, right?

If I understand the isApplicable() logic correctly, the platform info needs to checked before the legacy JUnit (3 + 4) infos. This is due to fact, that a legacy "junit-4.12.jar" is present in both cases: the users is on 4 and wants to run JUnit 4 test cases. Or the user is on "5" and wants to Vintage Engine to execute JUnit 4 or 3 test cases.

So, when we find org.junit.platform:junit-platform-engine as a dependency the uses wants to launch the JUnit Platform -- perhaps with the Vintage Engine which brings along JUnit 4.x jar.

AFAIK the found providers are run separately in a loop, see AbstractSurefireMojo.java in method execute.

sormuras · 2018-05-05T11:44:33Z

I think, we may get rid of integration test class JUnit5IT as it refers to the soon deprecated org.junit.platform:junit-platform-surefire-provider. Or should we keep it for the sake of completeness?

Tibor17 · 2018-05-05T17:19:38Z

@sormuras
Did you override the branch 1330? Because I squashed last cca 5 commits to one.
You have to squash all commits b83121..0633293 in one commit with this command
git rebase -i HEAD~11 amend the resulting commit message to
[SUREFIRE-1330] JUnit 5 surefire-provider code donation
and then force push itself branch
git push origin +1330:1330

Tibor17 · 2018-05-05T17:33:34Z

I will have a look to the sniffer plugin in the evening and I will try to run the build.

sormuras · 2018-05-05T18:21:49Z

... You have to squash all commits ...

Losing history and reasoning is okay?

Done.

https://gitbox.apache.org/repos/asf?p=maven-surefire.git;a=commit;h=fb1188b6b45e7a2d71e9d9807c67775a9595031d

sormuras · 2018-05-05T18:52:13Z

Why are they banned?

[linux-jdk10-maven3.5.x] Found Banned Dependency: org.apache.commons:commons-lang3:jar:3.7
[linux-jdk10-maven3.5.x] Found Banned Dependency: commons-io:commons-io:jar:2.6

Reverting to old versions...

sormuras · 2018-05-05T19:20:26Z

JDK 10 seems okay now. JDK 7 still chokes on "--target 1.8" options...

https://builds.apache.org/job/maven-box/job/maven-surefire/job/1330/8/

Tibor17 · 2018-05-05T23:00:40Z

@sormuras
Did you also have a problem with JaCoCO:0.7.9 on JDK10?
Do we need to have scope=compile on the dependency <artifactId>junit-platform-launcher</artifactId> in our provider POM? What will happen if it would be scope provided?

Tibor17 · 2018-05-05T23:03:40Z

@sormuras
How did you skip tests on surefire-junit-platform with JDK 1.7?

Tibor17 · 2018-05-05T23:08:01Z

Now I see, you used the build process JDK
<jvm>${java.home}/bin/java</jvm>
Very well!

Tibor17 · 2018-05-05T23:08:50Z

That's exactly why we have two JDKs.

sormuras · 2018-05-06T07:14:43Z

Did you also have a problem with JaCoCO:0.7.9 on JDK10?

According to https://github.com/jacoco/jacoco/releases 0.8.1 introduced JDK 10 support. Thus, 0.7.9 had to fail.

Do we need to have scope=compile on the dependency junit-platform-launcher in our provider POM? What will happen if it would be scope provided?

99% yes, the provider needs it at compile time. That's what JUnit Platform is all about. It's the API for tools (build tools, IDEs, ...) to get "JUnit 5" (technically, there's no JUnit 5) running. See this import statements in org.apache.maven.surefire.junitplatform.JUnitPlatformProvider:

import org.junit.platform.launcher.Launcher;
import org.junit.platform.launcher.LauncherDiscoveryRequest;
import org.junit.platform.launcher.TagFilter;
import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;
import org.junit.platform.launcher.core.LauncherFactory;

And recall the "JUnit 5" architecture layout:

Just move the junit-platform-surefire-provider from the bottom-left out off the red PLATFORM box into the IDEs/Build Tool bubble at the bottom. Here be dragons, Gradle, Surefire, IDEA, Eclipse, etc...

1% no, perhaps there's some Maven-trick to achieve the same with provided scope?

How did you skip tests on surefire-junit-platform with JDK 1.7?

Mh, if it is because of <jvm>${java.home}/bin/java</jvm> then @britter did it.
At test runtime, the assumption declared in JUnitPlatformIT ensures that a JDK 1.8+ is at work:

@Before
public void setUp()
{
    assumeThat( "java.specification.version: ",
                    getProperty( "java.specification.version" ),
                    is( greaterThanOrEqualTo( "1.8" ) ) );
}

Tibor17 · 2018-05-06T07:42:10Z

I think it will work even without is( greaterThanOrEqualTo( "1.8" ) ). I have initially designed the Jenkinsfile with two JDK which is why we did not use the build that Maven core uses. Now we can utilize and run the unit tests on the top of JDK 1.8. The build process always runs with JDK 1.8. The unit and integration tests run with JDK 1.7 - 10 except for JUnit5 Provider and therefore the condition in setUp() should be always satisfied.

…

On Sun, May 6, 2018 at 9:14 AM, Christian Stein ***@***.***> wrote: Did you also have a problem with JaCoCO:0.7.9 on JDK10? According to https://github.com/jacoco/jacoco/releases 0.8.1 introduced JDK 10 support. Thus, 0.7.9 had to fail. Do we need to have scope=compile on the dependency junit-platform-launcher in our provider POM? What will happen if it would be scope provided? 99% yes, the provider needs it at compile time. That's what JUnit Platform is all about. It's the API for tools (build tools, IDEs, ...) to get "JUnit 5" *(technically, there's no JUnit 5)* running. See this import statements in org.apache.maven.surefire.junitplatform. JUnitPlatformProvider: import org.junit.platform.launcher.Launcher;import org.junit.platform.launcher.LauncherDiscoveryRequest;import org.junit.platform.launcher.TagFilter;import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;import org.junit.platform.launcher.core.LauncherFactory; And recall the "JUnit 5" architecture layout: <https://user-images.githubusercontent.com/214207/38951423-e2d5c82c-4348-11e8-94f0-840a0c78b617.png> Just move the *junit-platform-surefire-provider* from the bottom-left out off the red *PLATFORM* box into the *IDEs/Build Tool* bubble at the bottom. Here be dragons, Gradle, Surefire, IDEA, Eclipse, etc... 1% no, perhaps there's some Maven-trick to achieve the same with provided scope? How did you skip tests on surefire-junit-platform with JDK 1.7? Mh, if it is because of <jvm>${java.home}/bin/java</jvm> then @britter <https://github.com/britter> did it. At test runtime, the assumption declared in JUnitPlatformIT ensures that a JDK 1.8+ is at work: @Beforepublic void setUp() { assumeThat( "java.specification.version: ", getProperty( "java.specification.version" ), is( greaterThanOrEqualTo( "1.8" ) ) ); } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#184 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_yR1mdMwmYlIekyWiJXr7QymRl2pwyks5tvqLlgaJpZM4TwvA6> .

-- Cheers Tibor

sormuras · 2018-05-06T07:51:49Z

I think it will work even without is( greaterThanOrEqualTo( "1.8" ) ).

Okay. Removed the setUp method. Let's see what Jenkins reports.

Tibor17 · 2018-05-06T07:55:01Z

It may happen that version of junit-platform-launcher would be too low after years?
Therefore I asked about different scope.
The junit-platform-launcher is dependent on junit-platform-engine which means for us that JUnit5 should guarantee backwards compatibility of platform engine for long time.

Tibor17 · 2018-05-06T07:57:32Z

@sormuras
Try to use your PC. The build takes few minutes with only your ITs.
It is just only about adding system property on commandline -Djdk.home=/path/to/jdk1.7.

sormuras · 2018-05-06T08:29:59Z

[...] JUnit5 should guarantee backwards compatibility of platform engine for long time.

That's exactly the plan of @junit-team! The platform is the stable part that build tools depend on. What a test is and how to execute it, is defined by an engine implementation. Like "Jupiter" now and "Saturn" (JUnit 6) in a few years... or any other flavor. See the list of well-known engine implementations at https://github.com/junit-team/junit5/wiki/Third-party-Extensions#junit-platform-test-engines

...so, if a build tool supports the JUnit Platform it doesn't have to care about new testing frameworks, as long as they implement TestEngine. The junit-platform-engine-jqwik IT in 1330 shows how and that it works.

Tibor17 · 2018-05-08T13:30:17Z

@sormuras
Regarding the plexus-java:0.9.8 I pushed the commit to the branch 1518 and additionally second commit in Jenkinsfile where I excluded machine windows-2012-3. See the Jenkins job status:
https://builds.apache.org/job/maven-box/job/maven-surefire/job/1518
We have 4 Windows machines altogether and excluded one which is not that bad but the build may take 3 hours to complete on master. The machine windows-2012-3 must have some problems with long Windows path because of the Compilation failures - the compiler does not reach the files. I repored this issue already to our INFRA team.

Tibor17 · 2018-05-08T13:42:14Z

@sormuras
I added JDK 11 b8 (early access build) to test on the branch 1518. So we should way especially for the outcome from JDK 11.

Tibor17 · 2018-05-08T15:41:36Z

I have reported a bug for PowerMock due to JDK 11
powermock/powermock#904
Another problem is on Windows:
16:38:38 [windows-jdk11-maven3.5.x] The system cannot find the path specified.
https://builds.apache.org/job/maven-box/job/maven-surefire/job/1518/8/consoleFull
So we have a real problem to test with JDK 11.

sormuras · 2018-05-08T15:46:49Z

I may look into the JDK 11 issues later... but as it is still early-access state, we and other tools have some months time to adopt.

Tibor17 · 2018-05-08T15:51:54Z

It seems we have to avoid one more machine, windows-2016-1, see
https://issues.apache.org/jira/browse/INFRA-16491
It is actually the most fast Windows machine ;-(

Tibor17 · 2018-05-08T16:22:48Z

https://issues.apache.org/jira/browse/INFRA-16500

sormuras · 2018-05-10T21:07:01Z

Rebased onto https://github.com/apache/maven-surefire/commits/master and merged gitbox-1330 changes.

Tibor17 · 2018-05-11T22:47:17Z

@sormuras
The CI https://builds.apache.org/job/maven-box/job/maven-surefire/job/1330/ does not see the branch after you have rebased 1330 on master. Did you push it to a new branch?

sormuras · 2018-05-12T03:56:10Z

No, didn't do it yet. Just wanted get the changes I made on gitbox-1330 to show up here as "Polishing". Will reset 1330 to this branch (github-184) and squash all commits into a single one there.

sormuras · 2018-05-12T04:34:08Z

What happend on the linux-jdk7-maven3.5.x build?

[linux-jdk7-maven3.5.x] Tests run: 58, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.858 sec
[linux-jdk7-maven3.5.x] 
[linux-jdk7-maven3.5.x] Results :
[linux-jdk7-maven3.5.x] 
[linux-jdk7-maven3.5.x] Tests run: 58, Failures: 0, Errors: 0, Skipped: 0
[linux-jdk7-maven3.5.x] 
[linux-jdk7-maven3.5.x] [INFO] 
[linux-jdk7-maven3.5.x] [INFO] --- maven-jar-plugin:3.0.0:jar (default-jar) @ surefire-booter ---
[linux-jdk7-maven3.5.x] [INFO] Building jar: /home/jenkins/jenkins-slave/workspace/ven-box_maven-surefire_1330-TSMLOBV5ENRMCK6YV52ANAULUWOG4OYEMBRTFOGSINEWTK45IYLQ/surefire-booter/target/surefire-booter-3.0.0-SNAPSHOT.jar
[linux-jdk7-maven3.5.x] [INFO] 
[linux-jdk7-maven3.5.x] [INFO] --- maven-site-plugin:3.4:attach-descriptor (attach-descriptor) @ surefire-booter ---
[linux-jdk7-maven3.5.x] [INFO] 
[linux-jdk7-maven3.5.x] [INFO] --- maven-shade-plugin:3.1.0:shade (default) @ surefire-booter ---
[linux-jdk7-maven3.5.x] [INFO] Excluding org.apache.maven.surefire:surefire-api:jar:3.0.0-SNAPSHOT from the shaded jar.
[linux-jdk7-maven3.5.x] [INFO] Excluding org.apache.maven.surefire:surefire-logger-api:jar:3.0.0-SNAPSHOT from the shaded jar.
[linux-jdk7-maven3.5.x] [INFO] Including org.apache.commons:commons-lang3:jar:3.5 in the shaded jar.
[linux-jdk7-maven3.5.x] [INFO] Including commons-io:commons-io:jar:2.5 in the shaded jar.
[linux-jdk7-maven3.5.x] [INFO] Minimizing jar org.apache.maven.surefire:surefire-booter:jar:3.0.0-SNAPSHOT
[linux-jdk7-maven3.5.x] [INFO] ------------------------------------------------------------------------
[linux-jdk7-maven3.5.x] [INFO] Reactor Summary:
[linux-jdk7-maven3.5.x] [INFO] 
[linux-jdk7-maven3.5.x] [INFO] Apache Maven Surefire .............................. SUCCESS [ 13.605 s]
[linux-jdk7-maven3.5.x] [INFO] SureFire Logger API ................................ SUCCESS [  3.933 s]
[linux-jdk7-maven3.5.x] [INFO] SureFire API ....................................... SUCCESS [ 12.695 s]
[linux-jdk7-maven3.5.x] [INFO] ShadeFire JUnit3 Provider .......................... SUCCESS [  1.921 s]
[linux-jdk7-maven3.5.x] [INFO] SureFire Booter .................................... FAILURE [ 10.408 s]
[linux-jdk7-maven3.5.x] [INFO] Maven Surefire Test-Grouping Support ............... SKIPPED
[...]

windows-jdk7-maven3.5.x build was doing fine.

sormuras · 2018-05-12T06:16:24Z

Restarting the build didn't help...

Tibor17 · 2018-05-12T06:18:42Z

See the log again. There must be a reason. Last days the compiler failed on linux and it was not our problem: TestsToRunTest.java:[113,4] error: error while writing TestsToRunTest.T1: could not create parent directories

…

On Sat, May 12, 2018 at 8:16 AM, Christian Stein ***@***.***> wrote: Restarting the build didn't help... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#184 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_yRyDOifjG86PzxFhk-Rbut434ei6Eks5txn45gaJpZM4TwvA6> .

-- Cheers Tibor

Tibor17 · 2018-05-12T06:23:32Z

I saw this error too and it happened because I triggered the branch 1338 and master in parallel: java.io.IOException: Failed to mkdirs: /home/jenkins/jenkins-slave/workspace/ven-box_maven-surefire_1330-TSMLOBV5ENRMCK6YV52ANAULUWOG4OYEMBRTFOGSINEWTK45IYLQ at hudson.FilePath.mkdirs(FilePath.java:1170) at org.jenkinsci.plugins.workflow.steps.CoreStep$Execution.run(CoreStep.java:79)

…

On Sat, May 12, 2018 at 6:34 AM, Christian Stein ***@***.***> wrote: What happend on the linux-jdk7-maven3.5.x <https://builds.apache.org/job/maven-box/job/maven-surefire/job/1330/> build? [linux-jdk7-maven3.5.x] Tests run: 58, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.858 sec [linux-jdk7-maven3.5.x] [linux-jdk7-maven3.5.x] Results : [linux-jdk7-maven3.5.x] [linux-jdk7-maven3.5.x] Tests run: 58, Failures: 0, Errors: 0, Skipped: 0 [linux-jdk7-maven3.5.x] [linux-jdk7-maven3.5.x] [INFO] [linux-jdk7-maven3.5.x] [INFO] --- maven-jar-plugin:3.0.0:jar (default-jar) @ surefire-booter --- [linux-jdk7-maven3.5.x] [INFO] Building jar: /home/jenkins/jenkins-slave/workspace/ven-box_maven-surefire_1330-TSMLOBV5ENRMCK6YV52ANAULUWOG4OYEMBRTFOGSINEWTK45IYLQ/surefire-booter/target/surefire-booter-3.0.0-SNAPSHOT.jar [linux-jdk7-maven3.5.x] [INFO] [linux-jdk7-maven3.5.x] [INFO] --- maven-site-plugin:3.4:attach-descriptor (attach-descriptor) @ surefire-booter --- [linux-jdk7-maven3.5.x] [INFO] [linux-jdk7-maven3.5.x] [INFO] --- maven-shade-plugin:3.1.0:shade (default) @ surefire-booter --- [linux-jdk7-maven3.5.x] [INFO] Excluding org.apache.maven.surefire:surefire-api:jar:3.0.0-SNAPSHOT from the shaded jar. [linux-jdk7-maven3.5.x] [INFO] Excluding org.apache.maven.surefire:surefire-logger-api:jar:3.0.0-SNAPSHOT from the shaded jar. [linux-jdk7-maven3.5.x] [INFO] Including org.apache.commons:commons-lang3:jar:3.5 in the shaded jar. [linux-jdk7-maven3.5.x] [INFO] Including commons-io:commons-io:jar:2.5 in the shaded jar. [linux-jdk7-maven3.5.x] [INFO] Minimizing jar org.apache.maven.surefire:surefire-booter:jar:3.0.0-SNAPSHOT [linux-jdk7-maven3.5.x] [INFO] ------------------------------------------------------------------------ [linux-jdk7-maven3.5.x] [INFO] Reactor Summary: [linux-jdk7-maven3.5.x] [INFO] [linux-jdk7-maven3.5.x] [INFO] Apache Maven Surefire .............................. SUCCESS [ 13.605 s] [linux-jdk7-maven3.5.x] [INFO] SureFire Logger API ................................ SUCCESS [ 3.933 s] [linux-jdk7-maven3.5.x] [INFO] SureFire API ....................................... SUCCESS [ 12.695 s] [linux-jdk7-maven3.5.x] [INFO] ShadeFire JUnit3 Provider .......................... SUCCESS [ 1.921 s] [linux-jdk7-maven3.5.x] [INFO] SureFire Booter .................................... FAILURE [ 10.408 s] [linux-jdk7-maven3.5.x] [INFO] Maven Surefire Test-Grouping Support ............... SKIPPED [...] windows-jdk7-maven3.5.x build was doing fine. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#184 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_yR7TRwQ2OhaXJ2mmrD0uA9avew1Srks5txmZBgaJpZM4TwvA6> .

-- Cheers Tibor

Tibor17 · 2018-05-16T19:15:54Z

maven-surefire-plugin/src/site/apt/featurematrix.apt.vm

+*---------------------------------------------+------------+----------+------------+-----------+----------+--------------------+
+| runOrder support                            |     Y      |    Y     |      Y     |    ?      |  Y       |  N                 |
+*---------------------------------------------+------------+----------+------------+-----------+----------+--------------------+
+| run >1 individual test method in a class    |     N      |    Y     |      Y     |    Y      |  N       |  ?                 |


Here we have question mark for run >1 individual test method in a class. I think this should be Y. WDYT?

Yes, but there are some corner cases that are not supported, yet (see junit-team/junit5#1343 and junit-team/junit5#1406).

Keeping the ? and adding the links as footnotes to the matrix.

Tibor17 · 2018-05-16T19:18:08Z

maven-surefire-plugin/src/site/apt/featurematrix.apt.vm

+*---------------------------------------------+------------+----------+------------+-----------+----------+--------------------+
+| run >1 individual test method in a class    |     N      |    Y     |      Y     |    Y      |  N       |  ?                 |
+*---------------------------------------------+------------+----------+------------+-----------+----------+--------------------+
+| parallel support                            |     N      |    N     |      Y     |    Y      |  N       |  N                 |


In parallel support we have N. Can we utilize ParallelComputer from JUnit4?
The PC covers parallelism. I would like to utilize it because it has a complex possibilities with configuration and I do not want to develop it again.

No, ParallelComputer cannot be reused. Jupiter currently does not have support for parallel execution, that will be added in junit-team/junit5#60.

@marcphilipp
It's better if we implement PC in surefire. We know what configuration we need. Is there something like Scheduler as it was in previous JUnit4? Is there a difference between runner and suite?

We should stick with 'N' for now.

Let's see which way is best to implement parallel execution support for the Platform.

Tibor17 · 2018-05-16T19:19:07Z

maven-surefire-plugin/src/site/apt/index.apt.vm

@@ -141,6 +141,8 @@ mvn verify

  * {{{./examples/testng.html}Using TestNG}}

+  * {{{./examples/junit-platform.html}Using JUnit Platform}}


Not sure if users would realize this is JUnit5 provider.

Can we just mention Using JUnit5 Platform - added 5.

Tibor17 · 2018-05-16T19:19:46Z

maven-surefire-plugin/src/site/site.xml

@@ -40,6 +40,7 @@
    <menu name="Examples">
      <item name="Using TestNG" href="examples/testng.html"/>
      <item name="Using JUnit" href="examples/junit.html"/>
+      <item name="Using JUnit Platform" href="examples/junit-platform.html"/>


Perhaps here as well.

Tibor17 · 2018-05-16T19:23:58Z

maven-surefire-plugin/src/site/apt/examples/junit-platform.apt.vm

+            <configuration>
+                <properties>
+                    <configurationParameters>
+                        junit.jupiter.conditions.deactivate = *


The separator is \n? Is it reliable?
What about CSV?
We should check if we put the value between "..." on CLI.

It uses Properties.load() so it will do the right thing whichever line separator is used.

Java code Properties.load() is okay but the way it is transferred via CLI might not be okay.

Do you mean transferring them from the Maven process to the forked VM? Or when specifying the property using -DconfigurationParameters? Or something else?

@marcphilipp
Yes I "mean transferring them from the Maven process to the forked VM".
We should check how this data is escaped and transfered.
Next thing, the text in POM should be in CDATA.

We should check how this data is escaped and transfered.

Agreed, but it works. 😉

Next thing, the text in POM should be in CDATA.

As long as the string does not contain any XML-specific characters, I don't see a reason to wrap it in a CDATA block. Am I missing something?

I think the newlines would be platform specific. We should check it in the code but I do not have time.

Where can I find the escape and transfer code in CLI @Tibor17 ?

As long as newline separators are transfered (doesn't matter which one is received on the forked VM) the loading via Properties.load will be fine.

Hi @sormuras. I will participate again tomorrow.

Tibor17 · 2018-05-16T19:25:49Z

maven-surefire-plugin/src/site/apt/examples/junit-platform.apt.vm

+            <version>{surefire-version}</version>
+            <configuration>
+                <properties>
+                    <includeTags>acceptance | !feature-a</includeTags>


Why we did not utilize Surefire's parameters groups and excludedGroups?
The same is in TestNG.

I think we should indeed do that, cf. junit-team/junit5#1425 (comment)

I'll take a look.

Do you mean with

Surefire's parameters groups and excludedGroups

the TestNG specific contants in org.apache.maven.surefire.booter.ProviderParameterNames?

public static final String TESTNG_EXCLUDEDGROUPS_PROP = "excludegroups"; public static final String TESTNG_GROUPS_PROP = "groups";

If yes, then I'd like to keep the JUnit 5 Platform configuration separate from those. The meaning of entries listed in groups and excludedGroups may be different.

Yes we have to use groups and excludegroups. I think this can be solved fast. The includeTags looks like system property. The provider observes these properties in constructor.

The only missing information is the mechanism how you can set includeTags via JUnit5 API.
This I do not know. We can write IT because it's the change and Surefire feature then.

The tags (include and exclude) are passed as filters to the org.apache.maven.surefire.junitplatform.TestPlanScannerFilter in line:

https://github.com/apache/maven-surefire/pull/184/files#diff-b3a2904e92be87f11f2622fbfd122e31R137

The filters are generated here:

https://github.com/apache/maven-surefire/pull/184/files#diff-b3a2904e92be87f11f2622fbfd122e31R172

Does that help?

@sormuras
Let's go to the IRC chat.

Tibor17 · 2018-05-16T19:30:58Z

maven-surefire-plugin/src/site/apt/examples/junit-platform.apt.vm

+---+
+if the JUnit Platform Engine is present in the project
+    use junit-platform
+if the JUnit version in the project >= 4.7 and the parallel attribute has ANY value


Pls emphase with <<<parallel>>> but the attribute should be rewritten to configuration parameter.

sormuras · 2018-06-05T08:29:19Z

...junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java

+    private boolean isTestSetStarted( TestIdentifier testIdentifier )
+    {
+        return testSetNodes.contains( testIdentifier )
+                        || testPlan.getParent( testIdentifier ).map( this::isTestSetStarted ).orElse( false );


The recursive call via map( this::isTestSetStarted ) may lead to a stack overflow error when an engine implementation provides a parent-child graph with cycles.

junit-team/junit5#1451 will fix this for the unreleased JUnit Platform version 1.3.0 -- creating an issue to track.

@sormuras
How did we come to this discussion with RunListenerAdapter.java? You wanted to make a fix in surefire for junit-team/junit5#1451?

No real engine will do that so I don't think it's crucial to introduce a (temporary) workaround in Surefire.

marcphilipp · 2018-06-07T15:41:06Z

Thanks @sormuras and @Tibor17 for getting this merged! I can't wait to use the released version! 🎉

olamy · 2018-06-07T21:07:37Z

Awesome guys!! Good job!!

eolivelli reviewed May 3, 2018

View reviewed changes

Tibor17 reviewed May 5, 2018

View reviewed changes

Tibor17 reviewed May 16, 2018

View reviewed changes

marcphilipp mentioned this pull request May 18, 2018

JUnit 5 with Maven Surefire does not run parallel tests junit-team/junit5#1424

Closed

marcphilipp mentioned this pull request May 26, 2018

Surefire can't find class org.apache.maven.plugin.surefire.log.api.ConsoleLogger junit-team/junit5#1367

Closed

[SUREFIRE-1330] JUnit 5 surefire-provider code donation

f379004

sormuras commented Jun 5, 2018

View reviewed changes

sormuras merged commit a132614 into apache:master Jun 7, 2018

sormuras deleted the new-junit5 branch June 10, 2018 10:43

trantienduchn mentioned this pull request Jul 27, 2018

JAMES-2495 Update various JUnit supporting libraries linagora/james-project#1577

Closed

		@@ -141,6 +141,8 @@ mvn verify

		* {{{./examples/testng.html}Using TestNG}}

		* {{{./examples/junit-platform.html}Using JUnit Platform}}

Donate current sources from junit-platform-surefire-provider #184

Donate current sources from junit-platform-surefire-provider #184

Conversation

sormuras commented May 3, 2018 • edited Loading

olamy commented May 3, 2018

eolivelli left a comment

Choose a reason for hiding this comment

Tibor17 commented May 3, 2018

sormuras commented May 3, 2018

Tibor17 commented May 3, 2018

sormuras commented May 4, 2018 • edited Loading

sormuras commented May 4, 2018

sormuras commented May 4, 2018

sormuras commented May 5, 2018

Tibor17 May 5, 2018 • edited Loading

Choose a reason for hiding this comment

sormuras May 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sormuras commented May 5, 2018

Tibor17 commented May 5, 2018

Tibor17 commented May 5, 2018

sormuras commented May 5, 2018 • edited Loading

sormuras commented May 5, 2018

sormuras commented May 5, 2018 • edited Loading

Tibor17 commented May 5, 2018

Tibor17 commented May 5, 2018

Tibor17 commented May 5, 2018 • edited Loading

Tibor17 commented May 5, 2018

sormuras commented May 6, 2018

Tibor17 commented May 6, 2018 via email

sormuras commented May 6, 2018

Tibor17 commented May 6, 2018

Tibor17 commented May 6, 2018

sormuras commented May 6, 2018

Tibor17 commented May 8, 2018

Tibor17 commented May 8, 2018

Tibor17 commented May 8, 2018

sormuras commented May 8, 2018

Tibor17 commented May 8, 2018

Tibor17 commented May 8, 2018

sormuras commented May 10, 2018

Tibor17 commented May 11, 2018

sormuras commented May 12, 2018 • edited Loading

sormuras commented May 12, 2018

sormuras commented May 12, 2018

Tibor17 commented May 12, 2018 via email

Tibor17 commented May 12, 2018 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tibor17 May 16, 2018 • edited Loading

Choose a reason for hiding this comment

sormuras commented May 3, 2018 •

edited

Loading

sormuras commented May 4, 2018 •

edited

Loading

Tibor17 May 5, 2018 •

edited

Loading

sormuras May 5, 2018 •

edited

Loading

sormuras commented May 5, 2018 •

edited

Loading

sormuras commented May 5, 2018 •

edited

Loading

Tibor17 commented May 5, 2018 •

edited

Loading

sormuras commented May 12, 2018 •

edited

Loading

Tibor17 May 16, 2018 •

edited

Loading

sormuras Jun 5, 2018 •

edited

Loading

Tibor17 Jun 7, 2018 •

edited

Loading