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