[SUREFIRE-2040] No tests executed with junit-platform-suite and -Dtest=TestSuite #494

Tibor17 · 2022-03-20T23:06:13Z

Following this checklist to help us incorporate your contribution quickly and easily:

Make sure there is a JIRA issue filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
Each commit in the pull request should have a meaningful subject line and body.
Format the pull request title like [SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles,
where you replace SUREFIRE-XXX with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Run mvn clean install to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
You have run the integration tests successfully (mvn -Prun-its clean install).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.

surefire-its/src/test/resources/junit5-suite/pom.xml

Tibor17 · 2022-03-22T22:36:16Z

@slawekjaranowski Feel free to have a look.

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

surefire-its/src/test/resources/junit5-suite/pom.xml

slawekjaranowski · 2022-03-23T06:43:14Z

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

+        TestListResolver filter = optionallyWildcardFilter( parameters.getTestRequest().getTestListResolver() );
+        if ( !filter.isEmpty() && !filter.isWildcard() )


Is it not the same?:

TestListResolver testListResolver = parameters.getTestRequest().getTestListResolver(); if ( testListResolver.hasMethodPatterns() ) ...

if true IT test doesn't cover this if with param <test>JUnit5Tests</test>

For such change we should add unit test in JUnitPlatformProviderTest
with assertions for filters ...

Another case you change logic here .. but all unit test still pass, looks like missing tests or assertions

@slawekjaranowski
No, it is not same. See the other Providers -> JUnit4Provider , JUnitCoreProvider and TestNGProvider
It is there sine 2015, i.e. 7 years.
We forgot to the same here? Just a mistake...

ok,

public static TestListResolver optionallyWildcardFilter( TestListResolver resolver ) { return resolver.hasMethodPatterns() ? resolver : WILDCARD; } public final boolean isWildcard() { return equals( WILDCARD ); }

so:

TestListResolver filter = optionallyWildcardFilter( resolver ); // this expression will always true - or I don't see something !filter.isWildcard() == resolver.hasMethodPatterns()

and is it possible?

resolver.hasMethodPatterns() == true && resolver.isEmpty() == true

We are again in the discussions with refactoring other providers. But this issue is about Junit5 provider. So my goal is not to make all these providers very different. I have made them similar in this PR through JunitPlatformProvider changes and nowhere else. I am not interested in others yet.
The optionallyWildcardFilter() can be removed but the condition can be changed in the next PR, i.e. !filter.isEmpty() && !filter.isWildcard() && !filter.hasMethodPatterns(). Here I am trying to make a fix and not to make providers too different in terms of filtering. Feel free to open a new PR after this and you can change all providers.

@slawekjaranowski
If you would work on the change, pls remove the method getWildcard() in TLR as well. It is not used.

ok, does not matter how and what we change in given expression, condition - such change should impact on unit tests.

I suppose that some of unit tests or assertions are missing, so should be fixed to be sure what we want to achieve.

If we sure that unit tests cover in proper way - the change has no effect and is not needed.

Unit tests exist in JUnitPlatformProviderTest

@slawekjaranowski
I guess JUnitPlatformProviderTest has similar use case which we can copy and adapt, this may help. This could be done in advance, your right. Please help me and push a new commit with the test if you have some spare time. Yesterday and today I am supporting Olivier with testing on a demo project. I would like to cut the release, the jdk 18 GA will be released soon as well and we have jdk 18 related fixes.
Ah, i forgot, we may use the latest MPOM, do you have experiences with the latest version?

Latest MPOM #491 ready and waiting for you 😄

As I understand getTestListResolver() returns TestListResolver with informations based on -Dtest param.

So please give me example like:

test=xxxx then filters.contains or filters.notContains some of TestMethodFilter instance or another filter instance which should be added for given -Dtest

then I will can write proper unit tests

As I understand getTestListResolver() returns TestListResolver with informations based on -Dtest param.

It also has some form of merged patterns includes/excludes, includeFile/excludeFile.
If you remember SUREFIRE-1426 with ildefonso and Olivier, that's exactly the part of code which updated the merge logic.

test=xxxx then filters.contains or filters.notContains some of TestMethodFilter instance or another filter instance which should be added for given -Dtest

Yes, I think the same, it would be enough to test newFilters() and verify the collection content. I found some example which would help you, and you can reuse it, see the test method onlyGroupsIsDeclared() in the test JUnitPlatformProviderTest.

slawekjaranowski · 2022-03-23T17:42:04Z

Remove of getWildcard #497

slawekjaranowski · 2022-03-24T11:58:03Z

ok, go with it ... after build fix 😄

I will prepare change with removing method optionallyWildcardFilter

slawekjaranowski · 2022-03-25T11:27:45Z

You can look at #499

Tibor17 · 2022-03-25T14:49:15Z

@slawekjaranowski
I think the #499 can be done after this PR as well.
What did we change in this PR since the build fails in compiler?

Error:  COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
Error:  /home/runner/work/maven-surefire/maven-surefire/surefire-its/target/Surefire1787JUnit5IT_junit5Suite/src/test/java/pkg/JUnit5Tests.java:[23,36] cannot find symbol
  symbol:   class Suite
  location: package org.junit.platform.suite.api

slawekjaranowski · 2022-03-25T15:06:15Z

I didn't touch this PR 😄
With #499 I will wait - discussion is pending.

Tibor17 · 2022-03-27T09:35:24Z

@slawekjaranowski
Pls see the unit tests I added in the last commit.
Can we go ahead with this PR?

slawekjaranowski · 2022-03-27T09:45:58Z

...latform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java

+        ProviderParameters providerParameters = providerParametersMock();
+        TestListResolver testListResolver = new TestListResolver( "*.java" );
+        assertTrue( testListResolver.isWildcard() );


ok *.java is wildcard ...
but **/*.java should be wildcard or not?

They are both the same in this case.
The TLR assumes wildcard path if not specified.

what result will be of?

new TestListResolver( '**/*.java' ). isWildcard()

...latform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java

Tibor17 · 2022-03-27T10:06:05Z

@slawekjaranowski
See the last commit.

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

…t=TestSuite

Tibor17 requested a review from slawekjaranowski March 21, 2022 00:13

davidmichaelkarr reviewed Mar 21, 2022

View reviewed changes

surefire-its/src/test/resources/junit5-suite/pom.xml Show resolved Hide resolved

asfgit force-pushed the SUREFIRE-2040 branch from 2c98551 to 1859027 Compare March 21, 2022 10:34

slawekjaranowski reviewed Mar 23, 2022

View reviewed changes

asfgit force-pushed the SUREFIRE-2040 branch 2 times, most recently from 93de677 to 088fad6 Compare March 23, 2022 10:26

slawekjaranowski reviewed Mar 27, 2022

View reviewed changes

...latform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java Outdated Show resolved Hide resolved

slawekjaranowski reviewed Mar 27, 2022

View reviewed changes

...latform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java Outdated Show resolved Hide resolved

slawekjaranowski reviewed Mar 27, 2022

View reviewed changes

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

slawekjaranowski approved these changes Mar 27, 2022

View reviewed changes

[SUREFIRE-2040] No tests executed with junit-platform-suite and -Dtes…

1f99455

…t=TestSuite

asfgit force-pushed the SUREFIRE-2040 branch from bce7dd6 to 1f99455 Compare March 27, 2022 19:41

Tibor17 merged commit a38bfcc into master Mar 27, 2022

Tibor17 deleted the SUREFIRE-2040 branch March 27, 2022 19:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SUREFIRE-2040] No tests executed with junit-platform-suite and -Dtest=TestSuite #494

[SUREFIRE-2040] No tests executed with junit-platform-suite and -Dtest=TestSuite #494

Tibor17 commented Mar 20, 2022 •

edited

Loading

Tibor17 commented Mar 22, 2022

slawekjaranowski Mar 23, 2022 •

edited

Loading

Tibor17 Mar 23, 2022 •

edited

Loading

slawekjaranowski Mar 23, 2022

Tibor17 Mar 23, 2022

Tibor17 Mar 23, 2022 •

edited

Loading

slawekjaranowski Mar 23, 2022

Tibor17 Mar 23, 2022 •

edited

Loading

slawekjaranowski Mar 23, 2022

slawekjaranowski Mar 23, 2022 •

edited

Loading

Tibor17 Mar 24, 2022 •

edited

Loading

slawekjaranowski commented Mar 23, 2022

slawekjaranowski commented Mar 24, 2022

slawekjaranowski commented Mar 25, 2022

Tibor17 commented Mar 25, 2022 •

edited

Loading

slawekjaranowski commented Mar 25, 2022

Tibor17 commented Mar 27, 2022

slawekjaranowski Mar 27, 2022

Tibor17 Mar 27, 2022

slawekjaranowski Mar 27, 2022

Tibor17 commented Mar 27, 2022

		TestListResolver filter = optionallyWildcardFilter( parameters.getTestRequest().getTestListResolver() );
		if ( !filter.isEmpty() && !filter.isWildcard() )

[SUREFIRE-2040] No tests executed with junit-platform-suite and -Dtest=TestSuite #494

[SUREFIRE-2040] No tests executed with junit-platform-suite and -Dtest=TestSuite #494

Conversation

Tibor17 commented Mar 20, 2022 • edited Loading

Tibor17 commented Mar 22, 2022

slawekjaranowski Mar 23, 2022 • edited Loading

Choose a reason for hiding this comment

Tibor17 Mar 23, 2022 • edited Loading

Choose a reason for hiding this comment

slawekjaranowski Mar 23, 2022

Choose a reason for hiding this comment

Tibor17 Mar 23, 2022

Choose a reason for hiding this comment

Tibor17 Mar 23, 2022 • edited Loading

Choose a reason for hiding this comment

slawekjaranowski Mar 23, 2022

Choose a reason for hiding this comment

Tibor17 Mar 23, 2022 • edited Loading

Choose a reason for hiding this comment

slawekjaranowski Mar 23, 2022

Choose a reason for hiding this comment

slawekjaranowski Mar 23, 2022 • edited Loading

Choose a reason for hiding this comment

Tibor17 Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

slawekjaranowski commented Mar 23, 2022

slawekjaranowski commented Mar 24, 2022

slawekjaranowski commented Mar 25, 2022

Tibor17 commented Mar 25, 2022 • edited Loading

slawekjaranowski commented Mar 25, 2022

Tibor17 commented Mar 27, 2022

slawekjaranowski Mar 27, 2022

Choose a reason for hiding this comment

Tibor17 Mar 27, 2022

Choose a reason for hiding this comment

slawekjaranowski Mar 27, 2022

Choose a reason for hiding this comment

Tibor17 commented Mar 27, 2022

Tibor17 commented Mar 20, 2022 •

edited

Loading

slawekjaranowski Mar 23, 2022 •

edited

Loading

Tibor17 Mar 23, 2022 •

edited

Loading

Tibor17 Mar 23, 2022 •

edited

Loading

Tibor17 Mar 23, 2022 •

edited

Loading

slawekjaranowski Mar 23, 2022 •

edited

Loading

Tibor17 Mar 24, 2022 •

edited

Loading

Tibor17 commented Mar 25, 2022 •

edited

Loading