On Tue, 9 May 2023 09:12:51 GMT, Afshin Zafari <d...@openjdk.org> wrote:
> This PR is continuation of https://github.com/openjdk/jdk/pull/13420 which > was far behind the master. The cleanups here look really good and solve the issue of the finalize function warnings. Any further refactoring or improvements to these tests should be a separate RFE. A couple of minor changes requested. Thanks! test/hotspot/jtreg/vmTestbase/nsk/share/FinalizableObject.java line 28: > 26: > 27: /** > 28: * This class is an simple exalmple of finalizable object, that typo: exalmple test/hotspot/jtreg/vmTestbase/nsk/share/LocalProcess.java line 170: > 168: * > 169: */ > 170: public void cleanup() { Maybe add `@Override` test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Binder.java line 755: > 753: if (vthreadMode) { > 754: /* Need --enable-preview on the debuggee in order to support > virtual threads. */ > 755: vmArgs += " --enable-preview"; Is this added with this change? It may have been recently removed which is why you have this diff. test/hotspot/jtreg/vmTestbase/nsk/share/jpda/BindServer.java line 155: > 153: > 154: registerCleanup(); > 155: remove the extra line pls. test/hotspot/jtreg/vmTestbase/nsk/share/jpda/BindServer.java line 410: > 408: * @see #close() > 409: */ > 410: public void cleanup() { Add `@Override` test/hotspot/jtreg/vmTestbase/nsk/share/jpda/DebugeeBinder.java line 321: > 319: args.add("--enable-preview"); > 320: } > 321: Also probably is an unintended diff with the master. ------------- Changes requested by coleenp (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13884#pullrequestreview-1418968684 PR Review Comment: https://git.openjdk.org/jdk/pull/13884#discussion_r1188815912 PR Review Comment: https://git.openjdk.org/jdk/pull/13884#discussion_r1188816875 PR Review Comment: https://git.openjdk.org/jdk/pull/13884#discussion_r1188818236 PR Review Comment: https://git.openjdk.org/jdk/pull/13884#discussion_r1188818689 PR Review Comment: https://git.openjdk.org/jdk/pull/13884#discussion_r1188818982 PR Review Comment: https://git.openjdk.org/jdk/pull/13884#discussion_r1188821265
