LOGMGR-308 add client.jvm.jpms.args property for specifying JPMS

[kstekovi]

default JPMS properties.

https://issues.redhat.com/projects/LOGMGR/issues/LOGMGR-308

[jamezp]

I'm not sure I follow this. We're already testing on Java SE 17 with no issues.

[kstekovi]

Customers are using a set of JPMS properties. And we need use same set for testing to have same settings. To make sure this new settings will not have any negative effects to behavior of this component.

[jamezp]

I still do not understand why these are needed. If it's working already with Java 17 and JPMS with testing, why do we need extra exports? We've already got an open PR to use a module-info.java which is a better approach IMO. It's just waiting until we can get releases of other projects which also use JPMS.

[kstekovi]

I get list of JPMS properties which have to be added surefire plugin. And I didn't know about the PR. @marekkopecky should know more why are needed.

[marekkopecky]

why do we need extra exports

Actually this MR doesn't add any extra exports, but the basic default set of recommended exports/opens. The point is to test in the same environment as users are using this library.

We also need to have consistent JPMS parameters in our TSs so new project (server/client) updates (e.g. new usage of JVM private packages, that was not used earlier in TS use-cases) can’t break our TS behavior.

Btw, WF itself is still not modularized using JPMS, i.e. it runs in what JPMS calls "classpath mode".

cc @OndrejKotek

[jamezp]

Those settings only make sense for WildFly though. Once @dmlloyd can update #362 there will be no need for those arguments. We are already passing tests without those settings so there is no need for them Adding opens and exports doesn't change whether or not tests will fail.

[marekkopecky]

Those settings only make sense for WildFly though.

WildFly and its clients in my PoV.

Once @dmlloyd can update #362 there will be no need for those arguments.

TBH I'm not sure about new @dmlloyd 's module-info plugin. Anyway, it seems that #362 doesn't use all parameters defined by the recommendation from @bstansberry . But module-info.yml files doesn't contain any packages from JDK (java.* / javax.* / etc) at all, so it seems that #362 doesn't relate with "--add-opens" and "--add-exports" parameters (or can you please share the details how does it relates?).

Anyway, would you be ok with adding empty parameter to the pom file, so defaults keep the same, but the TS would be parametrizable? Just keep empty <client.jvm.jpms.args></client.jvm.jpms.args> mvn property, that would be propagated to argLine parameter of surefire plugin.

[kstekovi]

I can update my PR to keep client.jvm.jpms.args empty. I'm waiting for approval if it can be like this.

[marekkopecky]

Anyway, would you be ok with adding empty parameter to the pom file, so defaults keep the same, but the TS would be parametrizable? Just keep empty <client.jvm.jpms.args></client.jvm.jpms.args> mvn property, that would be propagated to argLine parameter of surefire plugin.

@jamezp ^^^

[jamezp]

Sorry I missed that. I suppose an empty property is fine, but we really don't gain anything from adding properties like this. It won't indicate there are any errors and really might hide issues more than anything. I've been meaning to put an email together about this because there was a request for RESTEasy as well.

[dmlloyd]

The empty property seems OK to me, since it would allow external testing under a variety of conditions yet still keeps the default to empty which will avoid potential problems with testing where things are more open than a production environment might allow.

[kstekovi]

value removed from client.jvm.jpms.args