RFR: 8343977: Convert java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest to main [v3]

Alexey Ivanov aivanov at openjdk.org
Fri Nov 22 19:06:19 UTC 2024


On Tue, 19 Nov 2024 05:34:20 GMT, Tejesh R <tr at openjdk.org> wrote:

>> The test is supposed to be problem listed on macos & Linux but the Problem list points to the .java file instead of the .html file.
>> Hence converting java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest to main which would automatically resolve the issue.
>
> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Updated review comments

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java line 62:

> 60:                 .rows((int) INSTRUCTIONS.lines().count() + 2)
> 61:                 .columns(40)
> 62:                 .testUI(initialize())

Suggestion:

                .testUI(HoveringAndDraggingTest::initialize)

You should pass a method reference for the `PassFailJFrame` framework to create UI components on EDT, unless you need to create UI components on the main thread. See [this comment explaining the difference](https://github.com/openjdk/jdk/pull/21029#discussion_r1764872438).

test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java line 87:

> 85:     }
> 86: 
> 87:     static String bigString() {

`getLongString` could be a better name… which better fits in Java guidelines that methods are *verbs*.

test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java line 88:

> 86: 
> 87:     static String bigString() {
> 88:         String s = "";

It's better to use `StringBuilder` instead of `String` to create a long string.

test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java line 89:

> 87:     static String bigString() {
> 88:         String s = "";
> 89:         for (int lines = 0; ; ++lines) {

Suggestion:

        for (int lines = 0; lines < 50; ++lines) {

It's better to include the end condition into the `for`-loop rather than use `if`-block to break the loop.

test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java line 92:

> 90:             for (int symbols = 0; symbols < 100; ++symbols) {
> 91:                 s += "0";
> 92:             }

You can use [`repeat`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/String.html#repeat(int)) method instead of the loop.
Suggestion:

            s += "0".repeat(100);


Since 21, `StringBuilder` also has [`repeat` methods](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/StringBuilder.html#repeat(int,int)).

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

PR Review: https://git.openjdk.org/jdk/pull/22026#pullrequestreview-2455287698
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854512650
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854514473
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854516050
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854436611
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854498694


More information about the client-libs-dev mailing list