On Fri, 22 Aug 2025 05:51:38 GMT, Phil Race <p...@openjdk.org> wrote:
>> Leo Korinth has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update testing.md, remove makefile link, fix bad text > > test/jdk/javax/sound/sampled/Clip/AudioContentHandlers.java line 50: > >> 48: * @summary URL.getContent() should return SoundClip for supported >> formats >> 49: * @run main/othervm/timeout=480 -Xmx128m AudioContentHandlers >> 50: */ > > I've looked at our CI and this test has run 80,000 times and only 10 of those > have gone > 120 seconds (and only 2 > 145 seconds) > Perhaps I'd see similar for other tests. But I need to hear test-specific > reasons for the test-specific boost of 4x from what I think (120) is the > default to 480. > Otherwise I'd prefer no change, or a small change, by maybe 1.5x not 4x, and > we'll adjust the test when we see evidence that it is not enough. > @prrace the change maintains the same absolute timeout value for those tests. > Before the default of 120 was multiplied by the timeoutFactor of 4 to given > 480. Now the value 480 is multiplied by the timeoutFactor of 1 to give 480. > And IIRC Leo only did that for tests that demonstrated a timeout with the new > default settings (120*1). It is not practical for Leo to investigate every > changed test to see if it could get away with a value between 120 and 480. > The change just maintains the status quo. Test owners are free to investigate > further if they think it worth fine tuning these values. I don't agree. If you are going to modify individual tests, you need to demonstrate what you did for that test is justified or don't do it. I am also questioning whether such a time out was demonstrated for this test. I've searched the entire history of CI jobs and I don't see where Leo had such a timeout of this test. I can send you my query off-line so you can check it. Maybe it is incomplete. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2294258341
