Cleanup CRD generator tests

[Donnerbart]

Description

Cleanup CRD generator tests to current code style, e.g.

• use AssertJ
• use final var for local variables
• use List.of() and Map.of() for empty and singleton collections
• in general fix warnings (mostly missing Optional::isPresent checks in this case)

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change
• Chore (non-breaking change which doesn't affect codebase;
  test, version modification, documentation, etc.)

Checklist

• Code contributed by me aligns with current project license: Apache 2.0
• I Added CHANGELOG entry regarding this change
• I have implemented unit tests to cover my changes (n/a, it's just test code)
• I have added/updated the javadocs and other documentation accordingly (n/a, it's just test code)
• No new bugs, code smells, etc. in SonarCloud report
• I tested my code in Kubernetes (n/a, it's just test code)
• I tested my code in OpenShift (n/a, it's just test code)

[Donnerbart]

There was a test failure in the Windows Build / Java 17 Maven LeaderElectorTest.jitterWithPositiveShouldReturnPositiveDouble:338 expected: <true> but was: <false> which looks unrelated to my changes. Is this a known flaky test?

EDIT: Another failure that seems to be unrelated:

2024-11-25T21:11:21.6203975Z [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 8.939 s <<< FAILURE! -- in io.fabric8.openshift.client.impl.OpenShiftConfigTest
2024-11-25T21:11:21.6208738Z [ERROR] io.fabric8.openshift.client.impl.OpenShiftConfigTest.shouldInstantiateClientUsingSerializeDeserialize -- Time elapsed: 8.464 s <<< FAILURE!
2024-11-25T21:11:21.6210624Z org.opentest4j.AssertionFailedError: expected: <null> but was: <default>
2024-11-25T21:11:21.6211908Z 	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
2024-11-25T21:11:21.6213448Z 	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
2024-11-25T21:11:21.6214825Z 	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
2024-11-25T21:11:21.6215953Z 	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
2024-11-25T21:11:21.6217128Z 	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
2024-11-25T21:11:21.6218286Z 	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
2024-11-25T21:11:21.6220001Z 	at io.fabric8.openshift.client.impl.OpenShiftConfigTest.shouldInstantiateClientUsingSerializeDeserialize(OpenShiftConfigTest.java:90)
2024-11-25T21:11:21.6221855Z 	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
2024-11-25T21:11:21.6222803Z 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
2024-11-25T21:11:21.6223725Z 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

https://github.com/fabric8io/kubernetes-client/actions/runs/12018627620/job/33503520990?pr=6663

EDIT: Another failure that looks unrelated JdkHttpClientAsyncBodyTest>AbstractAsyncBodyTest.consumeBytesProcessesLargeBodies:115 » Execution java.net.ProtocolException: protocol error: zero streamId for DataFrame
https://github.com/fabric8io/kubernetes-client/actions/runs/12020938087/job/33510477323?pr=6663

[manusa]

Is this a known flaky test?

Yes.
There are a couple of flaky tests. Some have become more flaky now that everything is based on Vert.x.

[manusa]

Hi @Donnerbart
Thanks for looking into this.
A few comments I can already provide before digging into the actual review.

The changelog entry is not needed since this is just a test refactor and is not actually something relevant for the end users.

Multiple assertThat statements should be avoided.
I've seen this especially in your changes to the GeneratorTest where in most cases the multiple assertThat statements plus statisfies calls could be easily merged into a single statement.

[Donnerbart]

I've removed the changelog entry (just saw a similar one in a previous release).

@manusa Could you give me one concrete example how you would like the asserts to look like? I'm struggeling a bit with the deep nested structures and the many Optional<?> fields that have to be asserted.

How do you want to assert if you have to dive into multiple "branches" of the same object like in testClassNamesDisambiguationWithPackageNesting()? I'm not aware that e.g. extracing() can go back in the hierarchy for another assert. And with all the optionals I have no idea how something like hasFieldOrPropertyWithValue() could be used. Some variant of satifies() is the only way I know how to deal with these complex assertions.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[manusa]

@manusa Could you give me one concrete example how you would like the asserts to look like? I'm struggeling a bit with the deep nested structures and the many Optional<?> fields that have to be asserted.

Hi @Donnerbart,
I created #6689 which should provide you with some hints on how assertJ can be used (including the extraction and assertion) of optional values.

I will also check testClassNamesDisambiguationWithPackageNesting.

In general, if it's tricky to create an assertion with a single AssertJ statement it's a clear symptom that the test is checking too many things and should probably be split into multiple cases.

testClassNamesDisambiguationWithPackageNesting is a good example. In #6689 I split the test into multiple assertions since the original test was actually testing multiple things.

[manusa]

Also, considering the complexity of the Test suite, it might be better to create multiple PRs instead of a single one with all the changes at once.