RFR: 8328482: Convert and Open source few manual applet test to main based [v9]
Tejesh R
tr at openjdk.org
Fri Apr 5 04:35:26 UTC 2024
On Thu, 4 Apr 2024 17:20:14 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:
>> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Review comments updated - SizeMinimizedTest
>
> test/jdk/java/awt/Frame/SizeMinimizedTest.java line 37:
>
>> 35: * @bug 4065534
>> 36: * @summary Frame.setSize() doesn't change size if window is in an iconified state
>> 37: * @run main/manual SizeMinimizedTest
>
> manual should be removed since it is automated now.
> Does CI Testing look good with the updated test changes?
Yes, CI testing is good. Except linux which I have probemListed.
> test/jdk/java/awt/Frame/SizeMinimizedTest.java line 43:
>
>> 41: private static Frame frame;
>> 42: private static final int INITIAL_WIDTH = 100;
>> 43: private static final int INITIAL_HEIGHT = 100;
>
> Instead of defining width and height separately you can re-use the same constant by defining it as INITIAL_SIZE.
> Suggestion:
>
> private static final int INITIAL_SIZE = 100;
Done.
> test/jdk/java/awt/Frame/SizeMinimizedTest.java line 47:
>
>> 45: private static final int INITIAL_Y = 50;
>> 46: private static final int RESET_WIDTH = 200;
>> 47: private static final int RESET_HEIGHT = 200;
>
> Same here, can be renamed as one single constant RESET_SIZE.
Done.
> test/jdk/java/awt/Frame/SizeMinimizedTest.java line 58:
>
>> 56: Robot robot = new Robot();
>> 57: try {
>> 58: frame = new Frame("frame size test");
>
> Suggestion:
>
> frame = new Frame("Frame size test");
Done.
> test/jdk/java/awt/Frame/SizeMinimizedTest.java line 69:
>
>> 67: System.out.println("Initial Frame Location: " + frame.getLocationOnScreen());
>> 68: }
>> 69: });
>
> This windowlistener should be added before `frame.setVisible(true);` else the initial frame size and location are not logged.
> Moving the creation of UI to a separate method (eg. createUI) makes the code cleaner.
Done.
> test/jdk/java/awt/Frame/SizeMinimizedTest.java line 84:
>
>> 82: }
>> 83:
>> 84: expectedLoc = new Point(INITIAL_X + OFFSET * iterationCnt, INITIAL_Y);
>
> Please fix line-lengths.
Done.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18448#discussion_r1552862938
PR Review Comment: https://git.openjdk.org/jdk/pull/18448#discussion_r1552863068
PR Review Comment: https://git.openjdk.org/jdk/pull/18448#discussion_r1552863489
PR Review Comment: https://git.openjdk.org/jdk/pull/18448#discussion_r1552863171
PR Review Comment: https://git.openjdk.org/jdk/pull/18448#discussion_r1552863416
PR Review Comment: https://git.openjdk.org/jdk/pull/18448#discussion_r1552863268
More information about the client-libs-dev
mailing list