RFR: 7903961: Make default timeout configurable

[Jaikiran Pai]

On Fri, 7 Mar 2025 10:45:30 GMT, Christian Stein <cstein at openjdk.org> wrote:

> Please review this change replacing the hard-coded `120` seconds default timeout for test actions with a configurable solution parsing a custom default value from `TEST.ROOT` and `TEST.properties` files:
> 
> 
> timeout.default.seconds=360
> 
> 
> Especially in combination with "JUnit.dirs" and "TestNG.dirs", where all actions directives found in a test file are ignored, is helpful.

src/share/classes/com/sun/javatest/regtest/exec/RegressionScript.java line 1178:

> 1176:         if (isTimeoutsEnabled()) {
> 1177:             long seconds = properties.getDefaultTimeout(getTestDescription().getFile()).toSeconds();
> 1178:             p.put("test.timeout.default.seconds", Long.toString(seconds));

Is this the properties that get advertised to the application's test code? I'm guessing it is because I see that the `tag-spec` documentation has been updated to mention that this property will be made available to the test code. 

Is there a reason why the test code would be interested in this default value? As far as I can see we don't expose the final evaluated timeout value nor do we expose any other default configuration values (like the `defaultExecMode`) as properties to the test code. So it might be better not to expose this at least until it is really necessary.

src/share/classes/com/sun/javatest/regtest/exec/RegressionScript.java line 1357:

> 1355: 
> 1356:     private RegressionScript() {
> 1357:         this(null);

Does this constructor get used anywhere after this change? If yes, then we might have to add a null check at the call site where we are using the `properties` field to find the default timeout configuration value, like here https://github.com/openjdk/jtreg/pull/253/files#diff-19a61ca7f0fe24f891bee1b6b95f4ab2f740636458c9b20acff20c052aed9223R539

test/junitTrace/JupiterTimeoutTest.java line 38:

> 36:     void test() {
> 37:         String actual = System.getProperty("test.timeout.default.seconds", "-1");
> 38:         assertEquals("123", actual, "unexpected default value for timeout seconds");

It would be good to also include a test which overrides the default value in the test definition and verify that the overridden value is used. After all, that's the usage which prompted this change.

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

PR Review Comment: https://git.openjdk.org/jtreg/pull/253#discussion_r2022504451
PR Review Comment: https://git.openjdk.org/jtreg/pull/253#discussion_r2022507333
PR Review Comment: https://git.openjdk.org/jtreg/pull/253#discussion_r2022509584