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

[Jonathan Gibbons]

On Fri, 21 Oct 2022 21:43:04 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

> The fix adds support of a plugin that customizes test execution.

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

src/share/classes/com/sun/javatest/regtest/agent/CustomMainWrapper.java line 105:

> 103:         return vmOpts;
> 104:     }
> 105: }

Is this real code, or just toy test code?

src/share/classes/com/sun/javatest/regtest/config/RegressionParameters.java line 1248:

> 1246: 
> 1247:     private long timeoutHandlerTimeout;
> 1248:     //---------------------------------------------------------------------

You have not updated `load` and `save` in this class

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?

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

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.

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

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

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