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