<AWT Dev> Review request for JDK-8039279: Move first batch of functional tests to openjdk repository

Phil Race philip.race at oracle.com
Tue Apr 15 18:26:04 UTC 2014


http://cr.openjdk.java.net/~yan/8039279/webrev.02/test/java/awt/Frame/SetMaximizedBounds/SetMaximizedBounds.java.html


import static sun.awt.OSInfo.*;

Why is this using sun.* API ?
In module mode this will be invisible.

And all its used for is this :-
if (getOSType() == OSType.WINDOWS)

You can do that with a getProperty() call.


   36  * @build ExtendedRobot jdk.testlibrary.Asserts

What is ExtendedRobot ? I can't find it.


And do we really need a library dependency just to
do equals for us ??

   73             assertEquals(bound, max,
   74                     "The bounds of the Frame equal to what"
   75                     + " is specified when the frame is in Frame.MAXIMIZED_BOTH state");

Could easily be replaced by
if (!bounds.equals(max)) throw new RuntimeException(...);

We should make the tests as easy to run standalone as possible
and avoid internal APIs.

-phil

On 4/15/2014 6:14 AM, Sergey Bylokhov wrote:
> Hi, Dmitriy.
> A few comments:
> SetMaximizedBounds.java:  please add macosx to the test coverage. It 
> does not work now but the fix is under review.
> ChangeGridSize.java/ ComponentPreferredSize.java: actionPerformed must 
> be volatile, all other constant can be final.
> ModifierRobotKeyTest.java: tempPress must be volatile, access to 
> modifierStatus[] and textStatus should be synchronized somehow.
> LockingKeyStateTest.java:I guess we have synchronization problem as well.
>
> Additionally, can you dispose all frames in the tests.
>
> On 4/15/14 3:33 PM, Dmitriy Ermashov wrote:
>> Hi all.
>>
>> Petr, thanks for review!
>>
>> Guys, could you also review this tests?
>> http://cr.openjdk.java.net/~yan/8039279/webrev.02/
>>
>
>
> -- 
> Best regards, Sergey.



More information about the awt-dev mailing list