On Tue, 10 Mar 2026 09:44:18 GMT, Jaikiran Pai <[email protected]> wrote:
> Can I please get a review of this test-only change which proposes to use > "othervm" for some of these tests that configure java.util.logging logger > handlers? > > As noted in https://bugs.openjdk.org/browse/JDK-8379477 some of these test > definitions don't explicitly require a othervm and thus can interfere with > one another when configuring logging handlers (for the same logger). The > tests themselves don't seem to do anything useful with those loggers or > logging handlers, so the logging configuration could infact be completely > removed. But after discussing about it, we decided to just switch them over > to othervm for now and also use ConsoleHandler in all these tests. > > tier testing completed successfully with this change. test/jdk/com/sun/net/httpserver/TaskRejectedTest.java line 48: > 46: * @bug 8169358 > 47: * @summary HttpServer does not close client connection when > RejectedExecutionException occurs. > 48: * @run main/othervm TaskRejectedTest Can we use `${test.main.class}` here and all other `@run` tags, please? test/jdk/com/sun/net/httpserver/TaskRejectedTest.java line 48: > 46: * @bug 8169358 > 47: * @summary HttpServer does not close client connection when > RejectedExecutionException occurs. > 48: * @run main/othervm TaskRejectedTest Can we also document the choice for `othervm`, since it is not self-evident? For instance, @comment Since loggers are modified, `othervm` is added to disallow the reuse of this JVM Note that this remark applies to multiple places in this PR. test/jdk/com/sun/net/httpserver/TaskRejectedTest.java line 58: > 56: private static final int TIMEOUT = 10000; // 10 sec > 57: > 58: private static final Logger logger = > Logger.getLogger(HttpServer.class.getPackage().getName()); It is very easy to refactor these back into a local variable, just like they were before, and take the wrong path. Would you mind documenting the rationale for this class field, please? Suggestion: /** Anchor to avoid the {@code Logger} instance get GC'ed */ private static final Logger logger = Logger.getLogger(HttpServer.class.getPackage().getName()); Note that this remark applies to multiple places in this PR. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30158#discussion_r2910719287 PR Review Comment: https://git.openjdk.org/jdk/pull/30158#discussion_r2910795133 PR Review Comment: https://git.openjdk.org/jdk/pull/30158#discussion_r2910768800
