JUnit Jupiter best practices - #988

The methods renaming are fine for me, but I'm surprised by the removal of the public keywords. I would agree if this was the JAR that we publish (for better encapsulation), but those tests are not published and those methods need to be accessed by JUnit. I know that since version 5, JUnit can invoke non-public methods. But if we modularize Maven (something that I hope to prototype after completion of full JPMS support in all required plugins), it will require that we add --add-opens options for all packages containing test classes. It should be easy with module-info-patch.maven, but this is nevertheless an unnecessary additional complexity. Keeping the methods public also has the advantage of making clear what we intend to be accessed by JUnit, and what we intend to keep as internal helper methods for the tests (e.g. fixtures).

Should we rerun the recipe, but without the removal of public keywords?

Same comment applies to apache/maven-jar-plugin#489, apache/maven-clean-plugin#279 and apache/maven-resources-plugin#427.

Other similar pull requests where the comment may apply (also repeated the previous ones for completeness):

• JUnit Jupiter best practices maven-jar-plugin#489
• JUnit Jupiter best practices maven-clean-plugin#279
• JUnit Jupiter best practices maven-resources-plugin#427.
• JUnit Jupiter best practices maven-install-plugin#370
• JUnit Jupiter best practices maven-jdeps-plugin#67
• JUnit Jupiter best practices maven-jdeprscan-plugin#61
• JUnit Jupiter best practices maven-dependency-analyzer#254
• JUnit Jupiter best practices maven-dependency-plugin#1548
• JUnit Jupiter best practices maven-dependency-tree#116

Most of JUnit 5 tests are written without public following JUnit guide ....
https://docs.junit.org/current/user-guide/#writing-tests

So if it will be a problem maybe we need discuss with JUnit team ...?

and https://docs.junit.org/current/user-guide/#writing-tests-classes-and-methods

Actually their guide already raised my point:

Another technical reason for making classes and methods public is to simplify testing on the module path when using the Java Module System.

Removing public is not really a problem, but an unnecessary complication in JPMS context. Therefore, the recommendation to omit public seems questionable to me.

Actually their guide already raised my point:

Another technical reason for making classes and methods public is to simplify testing on the module path when using the Java Module System.

Removing public is not really a problem, but an unnecessary complication in JPMS context. Therefore, the recommendation to omit public seems questionable to me.

Agree.

Thanks for the feedback - hope that Maven will be soon modularized and also support plugin modules.
Then we can run recipes to make it compatible. Would it also require some changes to the surefire plugin?

Full module support indeed require change to the surefire plugin. The state of plugins is documented there:

https://cwiki.apache.org/confluence/display/MAVEN/Full+Java+Modules+Support+-+Current+State

Note that there is nothing wrong about executing the recipes now. The comment was only about the public keyword, not the rest.