[jira] [Commented] (LANG-1491) Unit tests visibility is non-conforming with JUnit 5

["Peter Verhas (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LANG-1491?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16948681#comment-16948681 ] 

Peter Verhas commented on LANG-1491:
------------------------------------

There was some discussion about the visibility of the different tests.

There was a statement that we should not change the visibility of individual methods one-by-one for consistency: as for now all test classes and methods are `public`.

There was another statement not to change all the methods visibility because (among other reasons) new pull requests may come in with public test methods and it is hard manual work to review and refuse/ask modifications of  those pull-requests because of these. (Btw: that is also true if all the methods are public: you can have pull requests that use package protected tests.)

After a long day of meeting and architecture documentation I thought it would be fun to write a unit test that just checks if all the test classes and methods are `public` (or the other way around NOT public if you ever decide to have package private tests). As for now I do not want to create a pull request for this. The major part of the code is here, just for demo:

https://github.com/verhas/commons-lang/blob/testPublic/src/test/java/org/apache/commons/lang3/TestVisibilityTest.java

What perplexed me is that the result on the current main I forked 2019-10-09 afternoon (Europe):

{code}
org.opentest4j.AssertionFailedError: Method org.apache.commons.lang3.FunctionsTest#testAcceptBiConsumer is not public
Method org.apache.commons.lang3.FunctionsTest#testAcceptConsumer is not public
Method org.apache.commons.lang3.FunctionsTest#testAsRunnable is not public
Method org.apache.commons.lang3.FunctionsTest#testRunnable is not public
Method org.apache.commons.lang3.FunctionsTest#testAsConsumer is not public
Method org.apache.commons.lang3.FunctionsTest#testCallable is not public
Method org.apache.commons.lang3.FunctionsTest#testAsCallable is not public
Method org.apache.commons.lang3.FunctionsTest#testAsBiConsumer is not public
Class org.apache.commons.lang3.FunctionsTest should be public
Method org.apache.commons.lang3.time.FastDateParser_TimeZoneStrategyTest#testLang1219 is not public
Class org.apache.commons.lang3.time.FastDateParser_TimeZoneStrategyTest should be public
{code}

How should I attend to this problem? Would you like this test in a PR changing the currently non public test to be public? Or should I include this test testing package private into the PR that changes all test methods to package private? Or should I just let this go altogether?


{panel:title=Side Note}
As a side note: I understand that such a unit test is not a perfect solution. I suggest it as a temporary fix till a future release of CheckStyle will have a check that does something similar. My son has already started working on that (https://github.com/checkstyle/checkstyle/issues/7176).
{panel}


> Unit tests visibility is non-conforming with JUnit 5
> ----------------------------------------------------
>
>                 Key: LANG-1491
>                 URL: https://issues.apache.org/jira/browse/LANG-1491
>             Project: Commons Lang
>          Issue Type: Improvement
>          Components: lang.*
>    Affects Versions: 3.9
>            Reporter: Peter Verhas
>            Priority: Major
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> The used JUnit 5 library does not require that the unit test classes and methods be {{public}}.
> All these {{public}} keywords are only noise in the source code and have to be deleted. Otherwise, the code becomes inconsistent as the new and modified tests will be package protected as per required by the test library and the old tests remain public unnecessarily.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)