On Thu, 8 May 2025 14:51:24 GMT, Leo Korinth <lkori...@openjdk.org> wrote:
> This change tries to add timeout to individual testcases so that I am able to > run them with a timeout factor of 1 in the future (JDK-8260555). > > The first commit changes the timeout factor to 0.7, so that I can run tests > and test the change (it will finally be changed to 1.0 in JDK-8260555). The > next commit excludes some junit/testng tests where I can currently not change > the timeout factor (CODETOOLS-7903961). Both these commits will be reverted > before integrating the change. I will also apply copyright updates after the > review. > > In addition to changing the timeout factor, I am also using a library call to > parse the timeout factor from the java properties (I can not use the library > function everywhere as jtreg does not allow me to add @library notations to > non testcase files). > > My approach has been to run all test, and afterwards updating those that > fails due to a timeout factor. The amount of updated testcases is huge, and > my strategy has been to quadruple the timeout if I could not directly see > that less was needed (thus the timeout will be the same after JDK-8260555 is > implemented). 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 plown through testcases: > 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 > > Sometime in the future I will also fix: > 8260555: Change the default TIMEOUT_FACTOR from 4 to 1 > > for which I am awaiting: > CODETOOLS-7903961 that is fixed in jtreg 7.6, but we are still running 7.5.1+1 > > *After the review I will revert the two first commits, and update the > copyrights* My biggest concern with this change is that potentially any test that implicitly uses the default timeout, and which passes with no problem with a factor of 4, may now need to have an explicit timeout set due to the factor of 1. I see this change in a number of tests (default timeout of 120s expressed as a new timeout of 480s to maintain equivalence**), but how many times did you run the tiers? I fear that the gatekeepers will be playing timeout whack-a-mole once these changes are put in. ** though another option would be to update the jtreg default timeout instead. ------------- PR Comment: https://git.openjdk.org/jdk/pull/25122#issuecomment-2865099247
