commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Peter Verhas (Jira)" <j...@apache.org>
Subject [jira] [Commented] (LANG-1491) Unit tests visibility is non-conforming with JUnit 5
Date Thu, 10 Oct 2019 15:09:00 GMT

    [ 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)

Mime
View raw message