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

[Yuri Nesterenko]

On 04/15/2014 10:26 PM, Phil Race wrote:
> 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.

Personally, I agree with that and try not to use OSInfo.
However the author of the fix was forced to use it by
previous reviewers. It is considered a standard routine
by now. For jigsaw, we'll have to invent something
to accommodate that.

>
>
>    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 ??
Re: ExtendedRobot please see
https://bugs.openjdk.java.net/browse/JDK-8038631
and explanations of our objectives there.
It was discussed in this list and run through 7 cycles
of review.
In short, we have hundreds of existing tests depending
on very complex Robot extensions. We have to move all
of them to the regression suite. And exactly to get rid of
(even) binary dependencies we proposed a minimal extension
which is both useful and allows us not to rewrite the bulk
of old code whenever possible.
This portion of tests also is not written anew
(which would be easier, it seems) but ported from functional
suite.

>
>    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.

Hear, hear!
Other colleagues here have different opinions though.

-yan

>
> -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.
>