RFR: 7903373: Add ability to customize test execution using main wrapper plugin [v5]

[Leonid Mesnik]

On Fri, 28 Oct 2022 22:29:37 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

>> Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   empty lines removed.
>
> src/share/classes/com/sun/javatest/regtest/agent/CustomMainWrapper.java line 38:
> 
>> 36: import java.util.List;
>> 37: 
>> 38: public interface CustomMainWrapper {
> 
> Add a javadoc comment explaining the purpose of the interface

I added comments to the interface and methods which might be re-implemented.

> src/share/classes/com/sun/javatest/regtest/tool/Tool.java line 1223:
> 
>> 1221: 
>> 1222:         if (customMainWrapper != null) {
>> 1223:             CustomMainWrapper cmw = CustomMainWrapper.getInstance(customMainWrapper, customMainWrapperPathArg);
> 
> Why are you creating an instance of the main wrapper on the tool side?

Not needed anymore.

> src/share/classes/com/sun/javatest/regtest/tool/Tool.java line 1225:
> 
>> 1223:             CustomMainWrapper cmw = CustomMainWrapper.getInstance(customMainWrapper, customMainWrapperPathArg);
>> 1224:             testVMOpts.add("-D" + MainWrapper.MAIN_WRAPPER + "=" + customMainWrapper);
>> 1225:             testVMOpts.addAll(cmw.getAdditionalVMOpts());
> 
> Ah, OK, there's the possibility that the instance of the wrapper class might want to specify some VM options. That's somewhat circular.  https://en.wikipedia.org/wiki/Ouroboros

Removed. The options might be set using javaoption. No need to add this feature right now,.

> src/share/classes/com/sun/javatest/regtest/tool/Tool.java line 1230:
> 
>> 1228:         if (customMainWrapperPathArg != null) {
>> 1229:             testVMOpts.add("-D" + MainWrapper.MAIN_WRAPPER_PATH + "=" + customMainWrapperPathArg);
>> 1230:         }
> 
> It seems premature to be stashing values in the `testVMOpts` here.  
> 
> The more conventional way would be to pass these values through the `RegressionParameters` object to where the instance of the `RegressionScript` can decide whether to take these values into account for each individual test.

Fixed, the params are passed through RP to RS where are used by MainAction only.

> src/share/classes/com/sun/javatest/regtest/tool/i18n.properties line 174:
> 
>> 172: help.main.ignore.run.desc=Run the test, as though the @ignore tag were not present.
>> 173: help.main.l.desc=List the tests that would be executed instead of executing them.
>> 174: help.main.mw.arg=<classname>=[options]
> 
> using an `=` between classname and any options seems unusual

I removed options, not sure if they are needed. We could them as a separate argument later.

-------------

PR: https://git.openjdk.org/jtreg/pull/136