RFR: 7903961: Make default timeout configurable

[Jaikiran Pai]

On Tue, 1 Apr 2025 09:38:36 GMT, Christian Stein <cstein at openjdk.org> wrote:

>> src/share/classes/com/sun/javatest/regtest/config/TestProperties.java line 280:
>> 
>>> 278: 
>>> 279:                     // add the default test timeout value in seconds
>>> 280:                     defaultTimeout = Duration.ofSeconds(getInt("timeout.default.seconds", 120));
>> 
>> Hello Christian, the `TEST.ROOT` (and `TEST.properties`) appear to follow the camel case style property names for most of the configurations https://openjdk.org/jtreg/tag-spec.html#TEST_ROOT. For example, `defaultExecMode`, `maxOutputSize` and so on. To be consistent with that, maybe we should expect this to be `defaultTimeoutSeconds` instead of `timeout.default.seconds`?
>
> It's quite a mixture already. There's also `lib.build` and `external.lib.roots` as counter examples.
> 
> The idea was to synchronize the key name with the system property exposed to tests in `RegressionScript`.

I don't have a strong opinion about this naming in the `TEST.ROOT`/`TEST.properties`, because like you state it's already a mix of naming styles.

>> 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.
>
> jtreg already exports the following timeout-related system property:
>> `test.timeout.factor`	`TESTTIMEOUTFACTOR`	_The timeout factor to be applied to the default timeout for the test._
> https://openjdk.org/jtreg/tag-spec.html#testvars
> 
> Test code and also test frameworks may want to adjust their bevaviour according both of these values. For example, having a test method-bound timeout or an assertion-based timeout makes more sense when the test author may react on the outer boundaries communicated by jtreg.

Hello Christian, I had forgotten that the test timeout factor gets exposed to test code as a property. Having said that, the test timeout factor is the final determined value unlike this proposed default timeout property which is a default value and may not even be the actual timeout that's ultimately used by jtreg.

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

PR Review Comment: https://git.openjdk.org/jtreg/pull/253#discussion_r2024924990
PR Review Comment: https://git.openjdk.org/jtreg/pull/253#discussion_r2024923425