On Tue, 17 Sep 2024 23:40:49 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> MToolkit removed > > LGTM other than the minor inline review comments @honkar-jdk please rereview.. > test/jdk/java/awt/Focus/MinimizeNonfocusableWindowTest.java line 59: > >> 57: .instructions(INSTRUCTIONS) >> 58: .rows((int) INSTRUCTIONS.lines().count() + 5) >> 59: .columns(35) > > Looks better formatted with the following: > > Suggestion: > > .rows((int) INSTRUCTIONS.lines().count() + 1) > .columns(40) ok > test/jdk/java/awt/Focus/MinimizeNonfocusableWindowTest.java line 61: > >> 59: .columns(35) >> 60: .testUI(MinimizeNonfocusableWindowTest::createTestUI) >> 61: .build() > > Suggestion: > `positionTestUI()` has been recently added to PassFailJFrame > https://github.com/openjdk/jdk/pull/21023 > Since this test has multiple test UIs you can try the new option to position > them if it doesn't require much re-work. I will skip this one as it has already been positioned and I dont think the API will auto-position unlike testUI > test/jdk/java/awt/Focus/MinimizeNonfocusableWindowTest.java line 66: > >> 64: >> 65: private static List<Window> createTestUI() { >> 66: Panel panel = new Panel(); > > Unused variable can be removed. ok > test/jdk/java/awt/Focus/WindowDisposeFocusTest.java line 68: > >> 66: >> 67: static public Window test(String[] args) { >> 68: final JFrame frame = new JFrame(); > > Frame title is missing. > Does setting a frame size look better here although the test is more to do > with the dialogs? ok > test/jdk/java/awt/Focus/bug6435715.java line 78: > >> 76: synchronized (this) { >> 77: try { >> 78: wait(3000); > > Is large wait time required here? I think it can be reduced. reduced.. ------------- PR Comment: https://git.openjdk.org/jdk/pull/21008#issuecomment-2357409777 PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764335877 PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764336544 PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764335831 PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764336061 PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764335944
