On Tue, 9 Apr 2024 01:27:29 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> Can I please get a review of this test-only change which proposes to address 
>> the intermittent failures in 
>> `java/util/Properties/StoreReproducibilityTest.java`? This should address 
>> https://bugs.openjdk.org/browse/JDK-8329729.
>> 
>> These failures in `StoreReproducibilityTest` have been observed in higher 
>> tiers where the test tasks are launched with various JVM options, one of 
>> them being `-Xcomp`. The goal of the `StoreReproducibilityTest` is to verify 
>> that the content which the `java.util.Properties` code generates for a 
>> properties file and reproducible across multiple different runs/launches of 
>> an Java application. To do that it launches a test application (using `java` 
>> command) several times within the test (for different scenarios). That comes 
>> up to a combined total of 25 launches, for different scenarios. Normally 
>> each such launch of the `java` application completes within a second or two. 
>> 
>> Recently, we have been updating our tests to pass along the JVM options that 
>> were used for launching the test task, to the child processes that are 
>> launched from within the tests. That now means that these trivial small java 
>> application that this test launches several times will now be passed the 
>> `-Xcomp` option too (when the test task is launched with that option). It 
>> has been observed that when `-Xcomp` is used to launch those trivial 
>> applications from within the test, each such application takes around 30 
>> seconds to a minute to complete. This then causes the test to timeout.
>> 
>> Given the context of this test case, it's not necessary to run this test 
>> when `-Xcomp` is used. The commit in this PR add a `@requires` to disable 
>> this test when `-Xcomp` is present in the test task's JVM args. 
>> 
>> I've run this change in our CI and the test continues to run (and pass) when 
>> `-Xcomp` is absent and is skipped when it is present.
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains two additional 
> commits since the last revision:
> 
>  - merge latest from master branch
>  - 8329729: java/util/Properties/StoreReproducibilityTest.java times out

Looks fine. If you can want, the comment could be much shorter. It just has to 
say that the test launches several processes and is too just with -Xcomp.

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

Marked as reviewed by alanb (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18681#pullrequestreview-1988155446

Reply via email to