Upgrade to junit 5

[rnveach]

As discussed at #6914 (comment),

We should try to upgrade to junit 5.
https://junit.org/junit5/docs/current/user-guide/#migrating-from-junit4

[strkkk]

Migration plan, in my vision

• Add junit 5 to classpath
• Migrate IT tests
• Migrate main code tests for packages except "check"
• Migrate tests in "check" package
• Remove old dependencies from classpath (including mock libraries for junit4 and system-rules) (moved to Make powermock tests compatible with JDK17+ #7368)
• Set up parallel test execution (when possible) to speed up CI (delayed till we restart https://github.com/checkstyle/checkstyle/projects/5)
• Forbid usage of old junit by changing CustomImportRules check configuration

[strkkk]

I checked and these changes are actually huge.
Probably it should be split into some smaller subtasks.
For example, if we can run both junit4 and junit5 tests simultaneously, upgrade can be applied per-package basis.

[romani]

Please suggest plan for sequential upgrade, it is preferable, as it will simplify review.

We can upgrade to junit5 binaries, but still use old junit4 classes ? and when think about how to migrate to new junit classes.
You can update description of issue and define list of sub task to be done, so this issue will be some sort of umbrella.
You can see example at #6726 .

[pbludov]

IntelliJ IDEA has an inspection to perform migration to JUnit5 automagically:
https://blog.jetbrains.com/idea/2017/11/intellij-idea-2017-3-junit-support/

[strkkk]

IntelliJ IDEA has an inspection to perform migration to JUnit5 automagically:
https://blog.jetbrains.com/idea/2017/11/intellij-idea-2017-3-junit-support/

Thanks, this one will definitely help here. I will check what things it cannot solve.

[strkkk]

Please suggest plan for sequential upgrade, it is preferable, as it will simplify review

Done, see second post in this issue.

IntelliJ IDEA has an inspection to perform migration to JUnit5 automagically

If it will be helpful for someone, this item can be found in "inspections" section and was disabled by default on my machine.

[romani]

We have config for inspections, if it is useful in long run , we can activate it now

[romani]

fist step is merged.

[pbludov]

Minor related issue: #7305
As a workaround it is possible to add the annotation to the check config.

[pbludov]

Another issue for DesignForExtension check and @BeforeEach annotation.

[pbludov]

Status update: @Isolated annotation works fine except for OSX builds on 64+ core hosts. They may fail with
java.util.concurrent.RejectedExecutionException: java.util.concurrent.RejectedExecutionException: Thread limit exceeded replacing blocked worker somewhere in the surefire executor.

Failed build example: https://travis-ci.org/github/checkstyle/checkstyle/jobs/722516044

It looks like this is a bug in surefire or even in JDK. It is possible that somewhere the number of physical cores is taken instead of the number of logical cores. It's hard to say for sure yet. I will research this issue further.

[rnveach]

@pbludov Is all that is left on the upgrade to do parallel execution? (Noted at #6916 (comment))
If so, maybe we should move it to a new issue and close this?

[pbludov]

If so, maybe we should move it to a new issue and close this?

I'll re-check how stable the OSX parallel tests now. Perhaps the reason is that we need to update the JDK for the OSX CI. This is the only build that has problems with parallel execution. Or may be just disable parallel execution of tests for this setup.

If It fail, I'll make this a separate issue, and close this one.

[pbludov]

I must admit that OSX build is stable now. I restarted the build 10 times. None of them had any problems.

[pbludov]

Finally, make it fail
https://travis-ci.org/github/checkstyle/checkstyle/jobs/734693767#L437

ERROR] Errors: 

[ERROR]   java.util.concurrent.RejectedExecutionException: Thread limit exceeded replacing blocked worker

I'll start a new issue for this

[pbludov]

This bug does not affect local OSX builds.

[pbludov]

The issue is done. Powermock tests will be updated in #7368