[SUREFIRE-1711] Support @ParameterizedTest for JUnit 5 test reruns

[Col-E]

This change fixes the issue discussed in #245 where @ParameterizedTest were not supported by the JUnitPlatformProvider.

The fix is rather simple, moving from DiscoverySelectors.selectMethod to DiscoverySelectors.selectUniqueId. Included are additions to JUnitPlatformProviderTest showing this feature is now supported.

I have detailed why this is a good alternative to parsing the legacy name in a gist: "The case for UniqueId over Legacy Name "

As mentioned in the gist, using selectUniqueId allows us to undo the changes made to RunListenerAdapter.toClassMethodName that were made in #245, which I've also included in this PR.

[Tibor17]

LGTM, I am waiting for the build result.
And i want to ask you about the value of legacy name. Would it return something like human readable description that is unique between two sets of parameters passed to the method?
Example:
test(java.lang.String) is not unique
test(Hello [1]) should be unique in this class because the next call would be perhaps test(Hi [2])
I wrote a roadmap in #251 where we should rework StatelessXmlReporter and identify the combination of class and mehod by UniqueId. But this is too much work for this week. Rather for new version 3.0.0-M5.

[Col-E]

And i want to ask you about the value of legacy name. Would it return something like human readable description that is unique between two sets of parameters passed to the method?

The only difference is the identifier at the end. So in your example if your parameters were: { "Hi", "Hello" } then the legacy names would be:

• test(String)[1]
• test(String)[2]

The display names would be what you're looking for:

• [1] Hi
• [2] Hello

But this is too much work for this week. Rather for new version 3.0.0-M5

Sounds good to me

[Tibor17]

ok, that's also good alternative.
I will merge this with master.

Can you then prepare a PR with the legacy name on the top of this commit?
Pls write a new IT. Here FailFastJUnitIT is a good help and you can extend the AbstractFailFastIT rather than reinvent the wheel with new IT. Write your own path to the IT project, see unpack(<dir>) and the path to the IT project would be surefire-its/src/test/resources/<dir>.
It would be worth if you selectively run *Platform*IT tests in your own. Then it takes only several minutes to complete the whole build.

btw, And if we additionally transfer the parameters in SimpleReportEntry, some users may implement their own report xml/json/yaml in the future releases. We have alreay done an extension for this purpose.

[Tibor17]

@Col-E
I will implement skipAfterFailureCount in JUnit5 provider but it won't work properly without legacy name. The XML report needs to have unique names for generating isolated report entries.
Ping me if you have any problem wih legacy name.

[Col-E]

Can you then prepare a PR with the legacy name on the top of this commit?

For clarification, are you asking for using the legacy name in RunListenerAdapter where SimpleReportEntry are generated? That way the report would include parametrized indicators such as test(String)[1]?

[Tibor17]

Yes, this was my plan but we have to run the unit tests and Platform ITs because i am not sure if they would succeed. We may have a problem with the brackets but the tests will show us a problem.
Your opinion?

[Col-E]

Reporting still uses the method toClassMethodName. So as of right now the reporting format should be the same as it was. This PR's changes only helps the launcher choose which tests to rerun.

Since normal tests do not have brackets we can easily separate the normal tests from the parameterized tests by checking if the legacy name contains a [ or ]. Those characters will never appear in normal @Test legacy names.

If we want to show that different runs of @ParameterizedTest are unique in the xml report with the [number] there would be some extra handling, but not really anything that would change existing behavior for @Test reporting, unless I'm misinterpreting what the goal here is.

Before I do anything to change the report format I would like to have a minimal example of what the xml output would be for parameterized support just to be clear on the intended format.

[Tibor17]

ok, let's change the methodology.
Although the provider is able to run parameterized test, run mvn test with real POM (without rerun) and see what happens in the test summary on the console and the XML report in target/surefire-reports.
I guess you will see the same problem what @Seijan found in junit-team/junit5#1558

The problem with JUnit5 and the Adapter is that we never know what annotations are used. We only can see some strings and signatures and list of method arguments but that's basically all regarding the test method.

[Tibor17]

@Col-E
You should be facing strange behavior with the report. It's not strange but the implementation was never dedicated to JUnit5. The reporter thinks that it is still the same metod and therefore the second run is seen as rerun. It does not know that it is method [1] and [2] etc.
Therefore we are reworking the interprocess communication because this is the prerequisite to fire and handle more events than it was before, sign the phase of normal run and the phase of rerun. After this we can reimplement the report. We also need to have UniqueId instead of guessing the identity of method run by class and method name description. This is quite a lot and cannot be done in one release, therefore we split the work in to several milestones and we put ordinal bug fixes. See this roadmap #251. You will see there what i have described.

[Col-E]

public class AppTest {
	@Test
	public void pass() { }
	
	@Test
	public void fail() {
		Assertions.assertTrue(false);
	}
	
	static List<Object[]> params() {
	return Arrays.asList(
		new Object[] { "One" },
		new Object[] { "Two" },
		new Object[] { "Three" }  );
	}
	
	@ParameterizedTest 
	@MethodSource("params")
	public void parameterizedPassOnThree(String key) {
		Assertions.assertEquals("Three", key);
	}
	
	@ParameterizedTest 
	@MethodSource("params")
	public void parameterizedFailAlways(String key) {
		Assertions.assertEquals("None", key);
	}
}

Oh yeah... I see that now. But I also noticed something else:

Running with no config options - 3.0.0-M3

image

<testcase name="pass" classname="pk.AppTest" time="0"/>
<testcase name="parameterizedPassOnThree{String}[1]" classname="pk.AppTest" time="0.019">

Running with no config options - 3.0.0-SNAPSHOT

image

<testcase name="pass" classname="pk.AppTest" time="0.001"/>
<testcase name="parameterizedPassOnThree" classname="pk.AppTest" time="0.019">

The parameter-set-number used to be reported but no longer is... 🤔

[Tibor17]

The answer is in the history of the Adapter class.
I don't say it was better in previous version, nothing but a different.

[Seijan]

Is this already finished? I tried to run a test with rerun and it shows the result as a success even if the test fails in one of the cases. And it doesn't rerun the failing test, and I set the parameter for it to 3.

[Tibor17]

@Seijan
I guess you mean the combination of rerun and parameterized.
The rerun should work with pure tests but not yet with parameterized ones.

In this issue we only fixed our previous fault in the code.
It was merged and we then have discussed the next issue in reports, that's what you can see on the console and in the XML report.
We only need to guarantee unique identity of tests and then we should expect that the combination of rerun and parameteried tests would reun as expected. That's why @Col-E is working on it. Truly we should jump to another issue but we do not have issues in GH so we discuss it here.

[Tibor17]

@Col-E
Can we continue on making the test unique?

[Col-E]

Yeah, I've just been a bit busy recently with coursework.

[Tibor17]

@Col-E
I fixed the issue with unique test representation in https://issues.apache.org/jira/browse/SUREFIRE-1716
Thx for your effort.

[Col-E]

Sorry about not finishing everything up, my schedule should be clearing up for a bit soon for future contributions.

[Tibor17]

@Col-E
no problem. You are always welcome.

@Col-E
@jon-bell
hey guys, we started the release VOTE, you can participate too, Feel free to subscribe in the mailing list
https://lists.apache.org/thread.html/6b9987be2fbd4d82daf8aaabbd1d58dfa2a533598770e0423d0b5ee0@%3Cdev.maven.apache.org%3E

[Micky002]

I have tested the new rerun feature with parameterized tests. It generally looks good but there is a problem with the exported xml report files when running the tests in parallel. (Parallel feature from junit 5). The reports got mixed. Normally there are only test results of one test class per xml file. But now there are random testcase tags from other classes inserted in files which does not belong to this class. This causes our pipeline to report wrong test results.

CC: @Seijan

[Tibor17]

@Micky002
I did not mention this because simply parallelism is not fixed. It is non-trivial issue which is actually not about JUnit5 but the design of the mechanism how we handle the test events into the reporter. Yes, this means that we have to rework some code and it is not little. Therefore we are aiming for M5. As one can see there are milestone releases because these will rework the legacy code and that's the only way to fix some issues. It's not because we like doing programming, but it's because we are in the situation where nothing else may help.

[Micky002]

Sry i just found the roadmap and saw that exactly this will be done in M5.

Thanks for the info :)

[kishoretak]

@Tibor17 When using "surefire-junit-platform" as the provider, selecting a single parametrized test (junit4) doesn't work as mentioned here- http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#test
The same works fine with "surefire-junit47" provider, so one way to handle that is to add both the provider.

Using mvn test -Dtest=sampletests.JUnit4ParametrizedExampleTest#sampleTest[true] for below example works only when "surefire-junit47" provider is added .

package sampletests;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.util.Arrays;
import java.util.Collection;

@RunWith(Parameterized.class)
public class JUnit4ParametrizedExampleTest {

    final boolean b;

    @Parameterized.Parameters(name="{0}")
    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][]{
                {false},
                {true}
        });
    }

    public JUnit4ParametrizedExampleTest(boolean b) {
        this.b = b;
    }

    @Test
    public void sampleTest() {
        /* always succeed */
    }
}

===================================

But for the junit5 parameterized test I couldn't run a single Parameterised with any of the below options, any idea what I'm doing wrong here?

mvn test -Dtest=sampletests.JUnit5ParametrizedExampleTest#sampleTest[true]
mvn test -Dtest=sampletests.JUnit5ParametrizedExampleTest#sampleTest[1]

package sampletests;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

public class JUnit5ParametrizedExampleTest {
    @ParameterizedTest
    @ValueSource(booleans={true,false})
    void sampleTest(boolean b) {
    }
}