RFR: 8340784 : Remove PassFailJFrame constructor with screenshots

Alexey Ivanov aivanov at openjdk.org
Thu Jan 23 13:36:00 UTC 2025


On Tue, 24 Dec 2024 03:02:51 GMT, anass baya <duke at openjdk.org> wrote:

> Remove PassFailJFrame constructor with screenshots since no test is using it

Changes requested by aivanov (Reviewer).

Nearly good.

Please, also update the copyright year at the top of the file: 2024 → 2025.

Looks good.

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 407:

> 405:      * the default values of {@value #ROWS} and {@value #COLUMNS}
> 406:      * for rows and columns.
> 407:      * The screenshot feature is not enabled, if you use this constructor.

I think this line about the screenshot should be removed now.

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 458:

> 456:             invokeAndWait(() -> createUI(title, instructions, testTimeOut,
> 457:                     rows, columns));
> 458:         }

Please, keep the existing call to `invokeOnEDT`, just remove the last parameter from `createUI`:

Suggestion:

        invokeOnEDT(() -> createUI(title, instructions,
                                   testTimeOut,
                                   rows, columns));

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 473:

> 471:      * @see Builder Builder
> 472:      */
> 473: 

This blank line is redundant, follow the code style in the file where there's no blank line between the javadoc and the declaration.

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 482:

> 480:                                    rows, columns));
> 481:     }
> 482:     

No trailing whitespace is allowed. The `jcheck` bot has marked it as an error and blocker.

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 510:

> 508:                         builder.positionWindows
> 509:                                .positionTestWindows(unmodifiableList(builder.testWindows),
> 510:                                                     builder.instructionUIHandler));

Move the documentation to the constructor above.

https://github.com/openjdk/jdk/blob/26b8b1e71f231d4601737a295dfed1ffd39f9b42/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java#L449-L451

 Otherwise, you're removing the detailed documentation, which other constructors refer to. You may need to amend some sentences.

-------------

PR Review: https://git.openjdk.org/jdk/pull/22873#pullrequestreview-2534741147
Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22873#pullrequestreview-2537104573
Marked as reviewed by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22873#pullrequestreview-2537962780
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1907158578
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1905703607
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1907165044
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1907165979
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1905688661


More information about the client-libs-dev mailing list