Apply JUnit5 best practices

[timo-a]

This PR applies JUnit best practices (via OpenRewrite recipes). For ease of review the changes are grouped into separate commits:

1. Remove public modifier from test classes and methods
2. Extract inner class Employee that is used in another package into its own class. This way its original class TestObjectId which contains tests and several inner classes can have its public modifier removed.
3. Remove "test" prefix from test methods as that is no longer required
4. Adapt assert in JDKStringLikeTypeDeserTest::stackTraceElement. Not sure what the test does but apparently it references the name of the test method which has now changed.
5. Use static imports for Assertions, so you don't have Assertios.assertXY in the code all the time but assertXY directly. Also replace star import with explicit assertions where appropriate.
6. Make assertions more concise e.g. assertEquals(false, p.value) -> assertFalse(p.value)
7. Revert above conversion for array builder class where equals is not symmetric (it is compared with an actual array -> different type)
8. Use explicit asserts for exception instead of fails in a catch block.
9. Remove unused imports.

[cowtowncoder]

Instead of submitting a huge PR with all changes without any discussion, we'd need multiple smaller PRs WITH discussion. I do not necessarily agree with all changes and definitely will not merge this PR as-is.

Part of the problem here, too, is that it feels bit of change for sake of change. "If it ain't broke don't fix it".

As an example, I don't really see whether it matters if test classes are public or not: if they weren't (and need not), that's fine. But I also don't want to pollute change history with busy work of change them. Ditto for test method visibility (public or not public) As to test method naming: fine, we can go and remove "test" prefix over time if need be, but I don't see the point of automated clean up either, due to noise it creates. Part of this is that many of changes, mechanical tho they are, make merging from 2.18 to 2.19 more conflict-prone (and probably 2.19 -> 3.0/master). But also... Code reviewing changes other than test method rename & class modifier change is now much more difficult due to noise of more or less pointless changes.

Some of changes are of course fine: f.ex just quickly:

2. Extract inner class Employee...
3. Adapt assert in JDKStringLikeTypeDeserTest::stackTraceElement (sounds like collateral change; and yes, StackTrace would have method name)
4. Make assertions more concise e.g. assertEquals(false, p.value) -> assertFalse(p.value)
5. Remove unused imports.

others I'd have to check.

[cowtowncoder]

Also was hoping find a case of

8. Use explicit asserts for exception instead of fails in a catch block.

to see what this is. But due to size of PR wasn't able to see.

EDIT: I can just look at individual commits; this was mentioned in PR. My bad.

[JooHyukKim]

Part of the problem here, too, is that it feels bit of change for sake of change. "If it ain't broke don't fix it".

Thank you for your words. I didn't know how to turn my feelings into words.

[pjfanning]

I agree with @cowtowncoder. I think these changes have low value and the large PRs mean they are hard to review. If I was a malicious coder, submitting PRs like this would strike me as a great way to add dangerous code. I have mentioned this to the OP in other PRs but they disregard comments.

[JooHyukKim]

Are you trying to merge this, @cowtowncoder? If so, I can also review. Hopefully reviewing (at least) twice will help improve quality.

[cowtowncoder]

@JooHyukKim I actually took parts and merged them separately. Couldn't cherry-pick commits, but changes applicable small enough it'd be ok. Will add a note next on things I applied.

[cowtowncoder]

@timo-a So, I think aside from 2 biggest noise factors ((1) and (3)), most changes make sense. So I applied following commits:

• (2) Extracting Employee
• (5) parts of
• (6) parts of
• (8) parts of
• (9) parts of

[cowtowncoder]

Ok; most applicable changes merged, will close the PR. No need to modify 600+ classes for the rest.