On Wed, 13 Aug 2025 01:47:43 GMT, SendaoYan <s...@openjdk.org> wrote:

>> This changes the timeout factor from 4 to 1. Most of the changes add 
>> timeouts to individual test cases so that I am able to run them with a 
>> timeout factor of 0.7 (some margin to the checked in factor of one)
>> 
>> In addition to changing the timeout factor, I am also using a library call 
>> to parse the timeout factor from the Java properties (I cannot use the 
>> library function everywhere as jtreg does not allow me to add @library 
>> notations to non test case files).
>> 
>> My approach has been to run all tests, and afterwards updating those that 
>> fail due to the timeout factor. The amount of updated test cases is huge, 
>> and my strategy has been to quadruple the timeout if I could not directly 
>> see that less was needed. In a few places, I have added a bit more timeout 
>> so that it will work with the 0.7 timeout factor.
>> 
>> These fixes have been created when I have ploughed through test cases:
>> JDK-8352719: Add an equals sign to the modules statement
>> JDK-8352709: Remove bad timing annotations from WhileOpTest.java
>> JDK-8352074: Test MemoryLeak.java seems not to test what it is supposed to 
>> test
>> CODETOOLS-7903937: JTREG uses timeout factor on socket timeout but not on 
>> KEEPALIVE
>> CODETOOLS-7903961: Make default timeout configurable
>> 
>> After the review, I will update the copyrights.
>> 
>> I have run testing tier1-8. The last time with a timeout factor of 1 instead 
>> of 0.7.
>> 
>> I got 4 timing related faults:
>> 1) runtime/Thread/TestThreadDumpMonitorContention.java
>>    This is probably: https://bugs.openjdk.org/browse/JDK-8361370
>> 2) sun/tools/jhsdb/BasicLauncherTest.java
>>    I am unsure about this one, it has not failed on my runs before, even 
>> with a timeout factor of 0.7, maybe I was unlucky.
>> 3) gc/stress/TestReclaimStringsLeaksMemory.java
>>    I updated this to 480 seconds, I finish this fairly fast (~14s) on my not 
>> very fast computer, but the Macs that fail are old x86-based ones.
>> 4) sun/security/ssl/X509KeyManager/CertChecking.java
>>    This is a new test that I got on last rebase. I have added a timeout of 
>> 480 to it.
>> 
>> In addition to these four tests, I have another one 
>> "java/lang/ThreadLocal/MemoryLeak.java" that earlier failed with a timeout 
>> factor of 0.7 but did not fail in the last run. I will *not* update that 
>> test case, because the extra time spent is strange and should be looked at. 
>> I have created JDK-8352074 on that test case, and I will probably create 
>> another bug describing the timeout I got.
>> 
>> From the review of the cancelled "8356171: ...
>
> test/hotspot/jtreg/compiler/c1/TestPinnedIntrinsics.java line 25:
> 
>> 23: 
>> 24: /*
>> 25:  * @test
> 
> Should we need to update the copyright year for the touched files

`After the review, I will update the copyrights.`

It is IMO easier to review big changes without the noise.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2273410872

Reply via email to