Donate current sources from junit-platform-surefire-provider #184
Conversation
|
awesome!! |
|
@sormuras |
|
Sure. Go ahead. I am still waiting for the ASF git permissions. |
|
@sormuras |
|
Nice. But or if I comment the exclude for How do I fix those build errors? |
|
Working-around it for now by |
|
@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. |
|
Make it three commits. 🔱 |
| /** | ||
| * JUnitPlatformProvider. | ||
| * | ||
| * @since 1.0 |
There was a problem hiding this comment.
Pls put here another version. We can make release of the provider in e.g. 2.22.0.
There was a problem hiding this comment.
Fixing all JUnit-related versions to 2.22.0.
| @@ -1022,9 +1029,9 @@ private void createDependencyResolver() | |||
| Artifact junitDepArtifact = getJunitDepArtifact(); | |||
| return new ProviderList( new DynamicProviderInfo( null ), | |||
| new TestNgProviderInfo( getTestNgArtifact() ), | |||
| new JUnitPlatformProviderInfo( getJunitPlatformArtifact() ), | |||
There was a problem hiding this comment.
Here the order of provider-info was changed. Does it mean something important for functionality?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
AFAIK the found providers are run separately in a loop, see AbstractSurefireMojo.java in method execute.
|
I think, we may get rid of integration test class |
|
@sormuras |
|
I will have a look to the sniffer plugin in the evening and I will try to run the build. |
Losing history and reasoning is okay? Done. |
|
Why are they banned? [linux-jdk10-maven3.5.x] Found Banned Dependency: org.apache.commons:commons-lang3:jar:3.7 Reverting to old versions... |
|
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/ |
|
@sormuras |
|
@sormuras |
|
Now I see, you used the build process JDK |
|
That's exactly why we have two JDKs. |
According to https://github.com/jacoco/jacoco/releases 0.8.1 introduced JDK 10 support. Thus, 0.7.9 had to fail.
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 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
Mh, if it is because of @Before
public void setUp()
{
assumeThat( "java.specification.version: ",
getProperty( "java.specification.version" ),
is( greaterThanOrEqualTo( "1.8" ) ) );
} |
|
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
|
Okay. Removed the |
|
It may happen that version of |
|
@sormuras |
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 |
|
@sormuras |
|
@sormuras |
|
I have reported a bug for PowerMock due to JDK 11 |
|
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. |
|
It seems we have to avoid one more machine, windows-2016-1, see |
|
Rebased onto https://github.com/apache/maven-surefire/commits/master and merged gitbox-1330 changes. |
|
@sormuras |
|
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. |
|
What happend on the
|
|
Restarting the build didn't help... |
|
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
|
|
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
|
| *---------------------------------------------+------------+----------+------------+-----------+----------+--------------------+ | ||
| | runOrder support | Y | Y | Y | ? | Y | N | | ||
| *---------------------------------------------+------------+----------+------------+-----------+----------+--------------------+ | ||
| | run >1 individual test method in a class | N | Y | Y | Y | N | ? | |
There was a problem hiding this comment.
Here we have question mark for run >1 individual test method in a class. I think this should be Y. WDYT?
There was a problem hiding this comment.
Yes, but there are some corner cases that are not supported, yet (see junit-team/junit5#1343 and junit-team/junit5#1406).
There was a problem hiding this comment.
Keeping the ? and adding the links as footnotes to the matrix.
| *---------------------------------------------+------------+----------+------------+-----------+----------+--------------------+ | ||
| | run >1 individual test method in a class | N | Y | Y | Y | N | ? | | ||
| *---------------------------------------------+------------+----------+------------+-----------+----------+--------------------+ | ||
| | parallel support | N | N | Y | Y | N | N | |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
No, ParallelComputer cannot be reused. Jupiter currently does not have support for parallel execution, that will be added in junit-team/junit5#60.
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
We should stick with 'N' for now.
Let's see which way is best to implement parallel execution support for the Platform.
| @@ -141,6 +141,8 @@ mvn verify | |||
|
|
|||
| * {{{./examples/testng.html}Using TestNG}} | |||
|
|
|||
| * {{{./examples/junit-platform.html}Using JUnit Platform}} | |||
There was a problem hiding this comment.
Not sure if users would realize this is JUnit5 provider.
There was a problem hiding this comment.
Can we just mention Using JUnit5 Platform - added 5.
| @@ -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"/> | |||
| <configuration> | ||
| <properties> | ||
| <configurationParameters> | ||
| junit.jupiter.conditions.deactivate = * |
There was a problem hiding this comment.
The separator is \n? Is it reliable?
What about CSV?
We should check if we put the value between "..." on CLI.
There was a problem hiding this comment.
It uses Properties.load() so it will do the right thing whichever line separator is used.
There was a problem hiding this comment.
Java code Properties.load() is okay but the way it is transferred via CLI might not be okay.
There was a problem hiding this comment.
Do you mean transferring them from the Maven process to the forked VM? Or when specifying the property using -DconfigurationParameters? Or something else?
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I think the newlines would be platform specific. We should check it in the code but I do not have time.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Hi @sormuras. I will participate again tomorrow.
| <version>{surefire-version}</version> | ||
| <configuration> | ||
| <properties> | ||
| <includeTags>acceptance | !feature-a</includeTags> |
There was a problem hiding this comment.
Why we did not utilize Surefire's parameters groups and excludedGroups?
The same is in TestNG.
There was a problem hiding this comment.
I think we should indeed do that, cf. junit-team/junit5#1425 (comment)
There was a problem hiding this comment.
Do you mean with
Surefire's parameters
groupsandexcludedGroups
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
The tags (include and exclude) are passed as filters to the org.apache.maven.surefire.junitplatform.TestPlanScannerFilter in line:
The filters are generated here:
Does that help?
There was a problem hiding this comment.
@sormuras
Let's go to the IRC chat.
| +---+ | ||
| 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 |
There was a problem hiding this comment.
Pls emphase with <<<parallel>>> but the attribute should be rewritten to configuration parameter.
| private boolean isTestSetStarted( TestIdentifier testIdentifier ) | ||
| { | ||
| return testSetNodes.contains( testIdentifier ) | ||
| || testPlan.getParent( testIdentifier ).map( this::isTestSetStarted ).orElse( false ); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
junit-team/junit5#1451 will fix this for the unreleased JUnit Platform version 1.3.0 -- creating an issue to track.
There was a problem hiding this comment.
@sormuras
How did we come to this discussion with RunListenerAdapter.java? You wanted to make a fix in surefire for junit-team/junit5#1451?
There was a problem hiding this comment.
No real engine will do that so I don't think it's crucial to introduce a (temporary) workaround in Surefire.
|
Awesome guys!! Good job!! |
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