Enable to use java preview while executing built jar file. #893

jedla97 · 2023-10-02T17:08:56Z

Summary

Hi, this PR adding option run built jar with preview argument. This can be useful when someone trying the JEPs which are marked as preview. What I tried when using framework with quarkus dev mode it pass the property -D--enable-preview but when using standard mode the property is not picked so jar failing to execute.

Running mvn -B -fae clean install -Pframework,examples -Dvalidate-format -Dquarkus.platform.version="999-SNAPSHOT" -pl examples/database-mysql -D--enable-preview

Before this PR result in produced command java -D<some Quarkus properties> -jar quarkus-test-framework/examples/database-mysql/target/quarkus-app/quarkus-run.jar

After this PR result in produced command java --enable-preview -D<some Quarkus properties> -jar quarkus-test-framework/examples/database-mysql/target/quarkus-app/quarkus-run.jar

Please check the relevant options

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Dependency update
Refactoring
Breaking change (fix or feature that would cause existing functionality to change)
This change requires a documentation update
This change requires execution against OCP (use run tests phrase in comment)

Checklist:

Example scenarios has been updated / added
Methods and classes used in PR scenarios are meaningful
Commits are well encapsulated and follow the best practices

michalvavrik

There is one thing I don't understand - if we only enable preview feature with system properties, how are tests going to pass if the feature is not enabled. I suppose your main objective is https://github.com/quarkus-qe/quarkus-jdkspecifics/tree/main ? I wonder if it is really desirable to add preview specific tests, we have enough on our plate with testing of what is actually in JDK. If this is only for your personal testing, can't you instead execute it manually, e.g. package and then run it as any other app?

It's possible I'm missing something, please let me know.

Update: I saw quarkus-qe/quarkus-jdkspecifics#99 (comment), so I'll leave review on someone else.

michalvavrik · 2023-10-02T17:24:40Z

...in/java/io/quarkus/test/services/quarkus/ProdLocalhostQuarkusApplicationManagedResource.java

@@ -25,6 +25,9 @@ protected List<String> prepareCommand(List<String> systemProperties) {
        String[] cmdArgs = extractQuarkusArgs(systemProperties);
        if (model.getArtifact().getFileName().toString().endsWith(".jar")) {
            command.add(JAVA);
+            if (isEnabledPreview()) {
+                command.add("--enable-preview");


pls make --enable-preview constant

michalvavrik · 2023-10-02T17:25:02Z

...in/java/io/quarkus/test/services/quarkus/ProdLocalhostQuarkusApplicationManagedResource.java

@@ -51,4 +54,14 @@ private String[] extractQuarkusArgs(List<String> systemProperties) {

        return args;
    }
+
+    private boolean isEnabledPreview() {


this method can be static

Removed as I using the PropertyLookup so this method is no longer needed

michalvavrik · 2023-10-02T17:35:06Z

...in/java/io/quarkus/test/services/quarkus/ProdLocalhostQuarkusApplicationManagedResource.java

@@ -51,4 +54,14 @@ private String[] extractQuarkusArgs(List<String> systemProperties) {

        return args;
    }
+
+    private boolean isEnabledPreview() {
+        String enablePreviewString = System.getProperty("--enable-preview");


I don't have strong opinion here, but if this is something specific for this framework. it should be framework config property. Therefore instead of -D--enable-preview create property like ts.enable-java-preview-features (please name it yourself) and use PropertyLookup and document it when this get merged in Wiki.

Let's here from others though, this is just my preference.

+1 for something like ts.enable-java-preview-features instead of -D--enable-preview

Should be documented in https://github.com/quarkus-qe/quarkus-test-framework/wiki/2.-Configuration once merged.

I play with it little and change it to the framework property.

I can see you two agreed on preview quarkus-qe/quarkus-jdkspecifics#99 (comment), so I take it back.

rsvoboda

This feature is specific for baremetal use case in prod mode / when application is started using java -jar.
It's OK-ish, we do not have a need for this in Kube/OpenShift environment or CLI / DEV mode at this moment.

We could consider more generic approach to allow definition of java command arguments, but could be a bit problematic for Kube/OpenShift and dev mode.

rsvoboda · 2023-10-02T19:40:07Z

...in/java/io/quarkus/test/services/quarkus/ProdLocalhostQuarkusApplicationManagedResource.java

@@ -51,4 +54,14 @@ private String[] extractQuarkusArgs(List<String> systemProperties) {

        return args;
    }
+
+    private boolean isEnabledPreview() {
+        String enablePreviewString = System.getProperty("--enable-preview");


+1 for something like ts.enable-java-preview-features instead of -D--enable-preview

Should be documented in https://github.com/quarkus-qe/quarkus-test-framework/wiki/2.-Configuration once merged.

mjurc

Generally, I agree with points made by @rsvoboda . I don't mind having it here, but maybe a more general approach could be used.

jedla97 · 2023-10-04T13:40:09Z

I play with this a little. At first I was thought that without -D--enable-preview the devmode won't be working and fail when running the test containing some preview feature. In my surprise there was no problem running it as it probably can take --enable-preview set in pom (this case in jdkSpecific TS).

Generally as this probably only quarkus-qe use case the framework property is better, but as just user who can possibly run it everywhere I would prefer -D--enable-preview as it propagated to all usecases/enviroments and it's defined in Java. Just my POV after thinking about it.

michalvavrik · 2023-10-04T14:56:18Z

I play with this a little. At first I was thought that without -D--enable-preview the devmode won't be working and fail when running the test containing some preview feature. In my surprise there was no problem running it as it probably can take --enable-preview set in pom (this case in jdkSpecific TS).

Generally as this probably only quarkus-qe use case the framework property is better, but as just user who can possibly run it everywhere I would prefer -D--enable-preview as it propagated to all usecases/enviroments and it's defined in Java. Just my POV after thinking about it.

I'll leave review to others as said, but if you want something propagated to all usecase/environments, can't you use JDK_JAVA_OPTIONS set in failsafe plugin? I take it we also need it during build so that is probably the issue, I won't dig into it...

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-failsafe-plugin</artifactId>
    <version>${failsafe-plugin.version}</version>
    <configuration>
        <environmentVariables>
            <JDK_JAVA_OPTIONS>--enable-preview</JDK_JAVA_OPTIONS>
        </environmentVariables>
    </configuration>
</plugin>

jedla97 · 2023-10-04T15:38:55Z

can't you use JDK_JAVA_OPTIONS set in failsafe plugin?

Basically yes in jdk-specific --enable-preview it set on multiple places as part of compilerArgs, jvmArgs or argLine and the locally it's used and executed without problem. Not tested it with Kube/OpenShift.

michalvavrik previously requested changes Oct 2, 2023

View reviewed changes

rsvoboda requested changes Oct 2, 2023

View reviewed changes

rsvoboda requested a review from mjurc October 2, 2023 19:46

mjurc reviewed Oct 2, 2023

View reviewed changes

Enable to use java preview while executing built jar file.

7849419

jedla97 force-pushed the java-enable-preview branch from 0ecf282 to 7849419 Compare October 4, 2023 13:23

rsvoboda approved these changes Oct 9, 2023

View reviewed changes

rsvoboda requested a review from mjurc October 9, 2023 08:49

mjurc approved these changes Oct 9, 2023

View reviewed changes

mjurc merged commit a24be04 into quarkus-qe:main Oct 9, 2023
7 checks passed

jedla97 deleted the java-enable-preview branch April 25, 2024 11:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable to use java preview while executing built jar file. #893

Enable to use java preview while executing built jar file. #893

jedla97 commented Oct 2, 2023 •

edited

Loading

michalvavrik left a comment •

edited

Loading

michalvavrik Oct 2, 2023

jedla97 Oct 4, 2023

michalvavrik Oct 2, 2023

jedla97 Oct 4, 2023

michalvavrik Oct 2, 2023

rsvoboda Oct 2, 2023

jedla97 Oct 4, 2023

rsvoboda left a comment

rsvoboda Oct 2, 2023

mjurc left a comment

jedla97 commented Oct 4, 2023

michalvavrik commented Oct 4, 2023

jedla97 commented Oct 4, 2023

Enable to use java preview while executing built jar file. #893

Enable to use java preview while executing built jar file. #893

Conversation

jedla97 commented Oct 2, 2023 • edited Loading

Summary

Checklist:

michalvavrik left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rsvoboda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjurc left a comment

Choose a reason for hiding this comment

jedla97 commented Oct 4, 2023

michalvavrik commented Oct 4, 2023

jedla97 commented Oct 4, 2023

jedla97 commented Oct 2, 2023 •

edited

Loading

michalvavrik left a comment •

edited

Loading