fixes Rerun failed test with maven and junit5 creates the wrong command line.

[homberghp]

This fix reformats the test property passed to the MavenCommandLineExecutor
by replacing occurrences of f.q.c.n.TestClass#f.q.c.n.TestClass.testMethod with
f.q.c.n.TestClass#testMethod.

Closes: #8533

[matthiasblaesing]

I had a look at this and I think this is the wrong location to fix this. Try these two runs:

1. run the full unittests and try to invoke "Run again" from the failing test t01Max
2. open RangeTest.java and run that single test:
   
   now try "Run again" in the test results window for that run.

The first run shows the problem you are observing, while the second does not. The difference between the two can be found in the JUnit Integration for maven:

netbeans/java/maven.junit.ui/src/org/netbeans/modules/maven/junit/ui/MavenJUnitTestMethodNode.java

Lines 72 to 82 in 1947efd

	if (actionProvider != null) {
	SingleMethod methodSpec = new SingleMethod(testFO, testcase.getName());
	Lookup nodeContext = Lookups.singleton(methodSpec);
	for (String action : actionProvider.getSupportedActions()) {
	if (unitTest
	&& action.equals(COMMAND_RUN_SINGLE_METHOD)
	&& actionProvider.isActionEnabled(action, nodeContext)) {
	actions.add(new TestMethodNodeAction(actionProvider,
	nodeContext,
	COMMAND_RUN_SINGLE_METHOD,

The critical section is line 73. For the first run returns io.github.jristretto.ranges.RangeTest.t01Max for testcase.getName() while the second run yields t01Max. The difference comes from here:

netbeans/java/junit/src/org/netbeans/modules/junit/api/JUnitTestcase.java

Lines 55 to 68 in 1947efd

	public String getName() {
	TestSuite currentSuite = getSession().getCurrentSuite();
	String className = getClassName();
	if (className == null || currentSuite == null) {
	return super.getName();
	}
	String suiteName = currentSuite.getName();
	// if the running suite is actually a test file return just the method name
	if(suiteName == null || suiteName.equals(className)) {
	return super.getName();
	}
	// the running suite is actually a suite, so return method's full path
	return className + "." + super.getName();
	}

For the full run you get a suiteName and thus you get the method name prefixed with the class. Case 1 goes through line 67, while case 2 goes through line 64.

Looking at the NB history (Link goes to a git clone of the original mercurial history):

emilianbold/netbeans-releases@3364af5

this strange behavior was introduces to handle nested tests and that does not even work correctly.

I get io.github.jristretto.ranges.SampleTest$NestedClass2.testMyMethod1 for the getName() call and io.github.jristretto.ranges.SampleTest$NestedClass2 for the getClassName() call. So you could just check in MavenJUnitTestMethodNode.java if class name is a prefix of name and if so strip it away. For the nested case more is required (check for $ and split them, but that is a different case and is already broken, so does not need to be fixed here).

[homberghp]

Thank you for your attention, and time and effort spent.
I will modify the PR accordingly.

Done 20225-06-10 13:56

[homberghp]

However, this solution does not cut it.
For the time being, I would keep the previous 'hacky fix'

[homberghp]

I had a look at this and I think this is the wrong location to fix this. Try these two runs:

1. run the full unittests and try to invoke "Run again" from the failing test t01Max
2. open RangeTest.java and run that single test:
   
   now try "Run again" in the test results window for that run.

The first run shows the problem you are observing, while the second does not. The difference between the two can be found in the JUnit Integration for maven:

netbeans/java/maven.junit.ui/src/org/netbeans/modules/maven/junit/ui/MavenJUnitTestMethodNode.java

Lines 72 to 82 in 1947efd

	if (actionProvider != null) {
	SingleMethod methodSpec = new SingleMethod(testFO, testcase.getName());
	Lookup nodeContext = Lookups.singleton(methodSpec);
	for (String action : actionProvider.getSupportedActions()) {
	if (unitTest
	&& action.equals(COMMAND_RUN_SINGLE_METHOD)
	&& actionProvider.isActionEnabled(action, nodeContext)) {
	actions.add(new TestMethodNodeAction(actionProvider,
	nodeContext,
	COMMAND_RUN_SINGLE_METHOD,

The critical section is line 73. For the first run returns io.github.jristretto.ranges.RangeTest.t01Max for testcase.getName() while the second run yields t01Max. The difference comes from here:

netbeans/java/junit/src/org/netbeans/modules/junit/api/JUnitTestcase.java

Lines 55 to 68 in 1947efd

	public String getName() {
	TestSuite currentSuite = getSession().getCurrentSuite();
	String className = getClassName();
	if (className == null || currentSuite == null) {
	return super.getName();
	}
	String suiteName = currentSuite.getName();
	// if the running suite is actually a test file return just the method name
	if(suiteName == null || suiteName.equals(className)) {
	return super.getName();
	}
	// the running suite is actually a suite, so return method's full path
	return className + "." + super.getName();
	}

For the full run you get a suiteName and thus you get the method name prefixed with the class. Case 1 goes through line 67, while case 2 goes through line 64.

Looking at the NB history (Link goes to a git clone of the original mercurial history):

emilianbold/netbeans-releases@3364af5

this strange behavior was introduces to handle nested tests and that does not even work correctly.

I get io.github.jristretto.ranges.SampleTest$NestedClass2.testMyMethod1 for the getName() call and io.github.jristretto.ranges.SampleTest$NestedClass2 for the getClassName() call. So you could just check in MavenJUnitTestMethodNode.java if class name is a prefix of name and if so strip it away. For the nested case more is required (check for $ and split them, but that is a different case and is already broken, so does not need to be fixed here).

I have tried what you suggested, but that appears not to cut it.
Reverted the commits in the PR back to my 'hacky solution'

Will create a better example project.

[homberghp]

Here an improved example project.
Note that the tests are intentionally broken.
The correct project can be found at https://github.com/jristretto/statewalker

statewalker.zip

Run test project, then rerun failed tests.
With this project the correctly built command line is:
mvn -Dtest=io.github.jristretto.statewalker.ContextBaseTest#testRawContext+testSomeMethod,io.github.jristretto.statewalkerpile.ContextBaseTest#testMethod+testMethod+testMethod+testMethod+testMethod+testMethod --no-transfer-progress test

[homberghp]

In the latets version I applied matthiasblaesing to a different file.
This does the trick.

[mbien]

@homberghp please don't mention issue numbers in titles or commit messages. To link a PR with an issue, all you have to do is to add:

closes #8533

to the PR message. github doc

This will link it properly, and won't spam additional links during branching etc. (also: the merge commit will already contain the PR ID - so this is already covered)

[homberghp]

will do.

[matthiasblaesing]

Looks sane to me and works in local tests. Thank you!

Minor nitpick for the future: Formatting should observe the existing format. In this case there are missing spaces in multiple places. See in this screenshot (JUnitOutputListenerProvider):

I will merge anyway as this is generally readable and I'd like to get this in now.

[matthiasblaesing]

@homberghp a second comment: when you force push into a branch, please also add a comment. Github does not notify about force pushes and so I missed your update.