fix [JDK17]kudu:integration test failed in native mode #3340

[ffang]

No description provided.

[zbendhiba]

@ffang can this be added in the same profile as jdk16-workarounds ?
there is a similar line here :

camel-quarkus/integration-tests/kudu/pom.xml

Line 113 in cb9c920

<argLine>--add-opens java.base/java.net=ALL-UNNAMED</argLine>

Maybe it will be good to refactor the config. WDYT ?

[ppalaga]

A custom argline is not an end user-friendly solution. Is there no Quarkus BuildItem for this?
If we keep relying on argline, then we need to document this on the given extension page,

[jamesnetherton]

A custom argline is not an end user-friendly solution.

I think in this case, it's acceptable because we unfortunately need this hack in the itests:

https://github.com/apache/camel-quarkus/blob/main/integration-tests/kudu/src/main/java/org/apache/camel/quarkus/component/kudu/it/KuduInfrastructureTestHelper.java#L64

It's not required for users to do this in their apps AFAIK.

[ffang]

Hi @zbendhiba ,

profile jdk16-workarounds has already had this configuration, however it couldn't take effect in native profile, since they both have maven-failsafe-plugin, but with different goals. So I think we have to add it into both profiles.

Freeman

@ffang can this be added in the same profile as jdk16-workarounds ? there is a similar line here :

camel-quarkus/integration-tests/kudu/pom.xml

Line 113 in cb9c920

<argLine>--add-opens java.base/java.net=ALL-UNNAMED</argLine>

Maybe it will be good to refactor the config. WDYT ?

[ffang]

A custom argline is not an end user-friendly solution.

I think in this case, it's acceptable because we unfortunately need this hack in the itests:

https://github.com/apache/camel-quarkus/blob/main/integration-tests/kudu/src/main/java/org/apache/camel/quarkus/component/kudu/it/KuduInfrastructureTestHelper.java#L64

It's not required for users to do this in their apps AFAIK.

Yes, this issue indeed from KuduInfrastructureTestHelper itself, which tries to access no-public field from java.net.InetAddress with reflection. This won't happen in users apps.

[ppalaga]

Thanks for the explanation @jamesnetherton and @ffang . Could we please have a big fat comment near the argLine saying that this is due to the nasty things we do in KuduInfrastructureTestHelper?

[ppalaga]

About where to put <argLine>--add-opens java.base/java.net=ALL-UNNAMED</argLine> given that java 11 is our baseline, where this should work and given that we need it for both plugins in all profiles, can't we just add

<properties>
  <!-- This is to allow the nasty things we do in KuduInfrastructureTestHelper -->
  <argLine>--add-opens java.base/java.net=ALL-UNNAMED</argLine>
</properties>

in the top profile-less context (and remove the old jdk16-workarounds profile altogether)?

[ffang]

About where to put <argLine>--add-opens java.base/java.net=ALL-UNNAMED</argLine> given that java 11 is our baseline, where this should work and given that we need it for both plugins in all profiles, can't we just add

<properties>
  <!-- This is to allow the nasty things we do in KuduInfrastructureTestHelper -->
  <argLine>--add-opens java.base/java.net=ALL-UNNAMED</argLine>
</properties>

in the top profile-less context (and remove the old jdk16-workarounds profile altogether)?

Hi @ppalaga ,

I may miss your point, but if we add

</dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <!-- This is to allow the "deep reflection" we do in KuduInfrastructureTestHelper -->
+                    <argLine>--add-opens java.base/java.net=ALL-UNNAMED</argLine>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>

in the pom.xml out of profiles, this can't affect the maven-failsafe-plugin configuration in native profile. The maven-failsafe-plugin configuration in native profile can't pick up this argLine and will fail with JDK17 in native mode. We need to add this argLine in native profile anyway.

Freeman

[ppalaga]

I spoke about <argLine> in <properties>, not plugin config. We should be able to hit both plugins that way.

[ffang]

I spoke about <argLine> in <properties>, not plugin config. We should be able to hit both plugins that way.

That's right, I will revise the PR soon.

Thanks!
Freeman

[ffang]

revised and squashed commits here
43d4560

[ppalaga]

Thanks @ffang!