Use existing Maven BootstrapSessionListener to hint Surefire about required JVM parameters

[Sanne]

As I make progress on #49920 , it's becoming clear that we'll need all JVM modes to start with some new JVM flags.
In some cases we can automate this transparently, in others it's harder - it seems likely that we'll end up with needing people to apply some minor migration steps to set necessary parameters.

Among all "modes" which I'm trying to cover - Surefire is particularly problematic.

This particular PR is an attempt to mitigate some such needs, by setting the required parameters to Surefire automatically.

N.B.

This is only effective if people use <extensions>true</extensions> in their pom.xml (Quarkus plugin config)

But at least those who do would have a better experience

I'm fairly annoyed by this limitation, but couldn't find a way to make it work universally and transparently. An alternative would be for people to add the required settings in .mvn/maven.config - but I'm inclined to pursue all such venues, so that at least those having Maven extensions enabled have some additional benefits deriving from it.

For these reasons this PR is independent from #49920 : it's not requiring it, and the other patch can't rely on it.
For those people not having extensions enabled, I'm afraid they'll have to add some JVM flags expicitly in their pom (to be defined, and documented, as I finish #49920).

Finally, since it might be detabable if we want to apply this - I'd rather debate this approach separately and in isolation from the larger work, so I'd start by merging this if there are no objections: should be harmless when it's not useful.

WDYT, @aloubyansky , @gsmet , @geoand ?

[aloubyansky]

I haven't tried this but reading How do I use properties set by other plugins in argLine?, i was wondering whether adding some generic quarkus-surefire-args property in the surefire-plugin configuration following @{} format and setting the value of quarkus-surefire-args in a quarkus-maven-plugin goal would work.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit f581063.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

[geoand]

This is only effective if people use true in their pom.xml (Quarkus plugin config)

If we do end up going down this route, we should at least log a message that we were not able to apply the proper flags automatically.

[Sanne]

This is only effective if people use true in their pom.xml (Quarkus plugin config)

If we do end up going down this route, we should at least log a message that we were not able to apply the proper flags automatically.

Yes of course, I plan warnings and proper deprecations.

I do have alternative ways to set the flags - it's just that I like them less (they imply the agent approach with self-attach, discussed on the other PR).
So I'm inclined to at least try to patch the majority of our runners to pre-set the flag correctly, and fallback to the agent only when they aren't set as required.

Essentially this gives us the opportunity to let people migrate to better approaches by nudging them rather than having a strong requirement, and gradually nudge them to better approaches.

In practice I think I'd like to get people to add <extensions>true</extensions> in their project because it's future proof: should we need to make further changes to the JVM flags, we can make them w/o people needing to adapt again.

[Sanne]

I haven't tried this but reading How do I use properties set by other plugins in argLine?, i was wondering whether adding some generic quarkus-surefire-args property in the surefire-plugin configuration following @{} format and setting the value of quarkus-surefire-args in a quarkus-maven-plugin goal would work.

I'm not following @aloubyansky , could you elaborate? Do you mean people should be adding this to their project? My goal was to figure out ways to set what people need w/o them having to change their build scripts.

[aloubyansky]

For a newly generated project, the surefire config looks like this currently:

            <plugin>
                <artifactId>maven-surefire-plugin</artifactId>
                <version>${surefire-plugin.version}</version>
                <configuration>
                    <argLine>--add-opens java.base/java.lang=ALL-UNNAMED</argLine>
                    <systemPropertyVariables>
                        <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
                        <maven.home>${maven.home}</maven.home>
                    </systemPropertyVariables>
                </configuration>
            </plugin>

What we could consider doing is introducing a project property, e.g. quarkus.surefire.argLine:

    <properties>
        <quarkus.surefire.argLine>--add-opens java.base/java.lang=ALL-UNNAMED</quarkus.surefire.argLine>
    </properties>
...
            <plugin>
                <artifactId>maven-surefire-plugin</artifactId>
                <version>${surefire-plugin.version}</version>
                <configuration>
                    <argLine>@{quarkus.surefire.argLine}</argLine>
                    <systemPropertyVariables>
                        <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
                        <maven.home>${maven.home}</maven.home>
                    </systemPropertyVariables>
                </configuration>
            </plugin>

So far, this would be equivalent to what we have. But @{} format allows us to set a new value for quarkus.surefire.argLine in a quarkus goal. For example,

$ git diff
diff --git a/devtools/maven/src/main/java/io/quarkus/maven/GenerateCodeMojo.java b/devtools/maven/src/main/java/io/quarkus/maven/GenerateCodeMojo.java
index d95e82c8902..4f8e0c7f0bc 100644
--- a/devtools/maven/src/main/java/io/quarkus/maven/GenerateCodeMojo.java
+++ b/devtools/maven/src/main/java/io/quarkus/maven/GenerateCodeMojo.java
@@ -118,6 +118,9 @@ void generateCode(PathCollection sourceParents, Consumer<Path> sourceRegistrar,
                         }
                     }
                 }
+
+                mavenProject().getProperties().setProperty("quarkus.surefire.argLine",
+                        "--add-opens java.base/java.lang=ALL-UNNAMED -Xmx1g");
             }
         }
     }

That way we have a specific Quarkus property to override, which has its pros and cons. It doesn't require enabling extensions but it does require running a quarkus:xxx goal.

We already do some optimization steps in generate-code-tests, such as resolve and persist the test ApplicationModel that would be loaded when bootstrapping tests. It's optional though.

[Sanne]

I see, thanks for the details! However I was really hoping for a solution in which we could control the Surefire arguments w/o the user needing to make changes to existing projects - it seems like that's not possible :-/

Now, having concluded that our users will need to make some changes: shouldn't we rather suggest users to enable pugin extensions? I'd much rather ask to add a single one-liner, which we can easily validate.

My hope is that it's more future proof: should we need further evolutions, including in other areas, we can make them transparently without having to ask users to make changes yet again.
Apparently we can also better verify / validate things, as we can read the whole structure, while with the double property above we need to pray people are doing it right.

This makes me think, actually, that I could already patch the extension today to check if extensions are enabled and suggest to do it when not.