On Tue, 23 Apr 2024 08:39:56 GMT, Christoph Langer <clan...@openjdk.org> wrote:
>> While working in that area I found some potential for cleanup of a few tests. >> >> Most notably: >> >> B5045306.java: >> - does not need to run in othervm >> - the executor service that it uses should be shut down eventually to free >> resources >> >> B8291637.java: >> - use just one instead of two test VM invocations >> KeepAliveTimerThread.java: >> call to grp.destroy() at the end is pointless (API is void & deprecated for >> removal) >> >> Generally: >> The deprecated URL constructor is used. It can be switched to the handy >> URIBuilder >> Some more try with resources here and there > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Update test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java > > Co-authored-by: Andrey Turbanov <turban...@gmail.com> test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 154: > 152: > 153: // if Keep-Alive-SocketCleaner consumes more than > 50% of cpu then we > 154: // can assume a recursive loop. Interesting test case. I'm a bit surprised we haven't seen this intermittently fail. Speaking of which, I see that the change to this test proposes to remove it from `/othervm` and potentially run it in agent vm mode. Given the kind of checks this test is doing, I think it's better to let it run as a `othervm` test to keep the test as isolated as possible. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1577380954