<AWT Dev> Review Request for 8039749: Migrate needed functionality from all subclasses of java.awt.Robot in jdk/test directory to the ancestor

[Yuri Nesterenko]

On 08/27/2014 04:25 PM, Dmitriy Ermashov wrote:
> Hi Anthony,
>
> Thanks for your review.
>
> At first, let me focus on fact that the actual motivation of moving
> functionality to java.awt.Robot is the Jigsaw project. We (SQE) will be
> unable to use internal java API after the project will be finished
> (ExtendedRobot just will not compile, for example) and it will be a
> reason of failing huge amount of regression and functional tests.
>
> As for waitForIdle() method, we do not change it's use-case. The
> java.awt.Robot class is used only for GUI actions. And waitForIdle() is
> used for ensuring of finishing all events in EventQueue.
> The implementation with realSync() just flushes native queue as well and
> it is just an improved version of existing one.
>
> Anyway, the changing of waitForIdle() implementation is discussable. The
> other solution may be in adding new method with realSync call.

Which would require touching some 340 tests in apprx 950 places, sorry
for statistics.

-yan

>
> Thanks,
> Dima
>
> On 08/27/2014 03:16 PM, Anthony Petrov wrote:
>> Hi Dmitriy,
>>
>> While I realize that all the new methods are useful when writing JDK
>> regression tests, do you have any evidence that would suggest that
>> these same methods could be useful to and/or have been requested by
>> external developers? All of them look like convenient APIs and I'm not
>> entirely convinced if we should ultimately add them all to our public
>> Robot API. Personally, I don't see a problem with using the custom
>> ExtendedRobot class specifically for tests. This also helps reduce the
>> JDK and JRE static footprints, btw. But then again, I'm not an SQE
>> engineer.
>>
>> I don't have a strong opinion on this and I'm leaving the final
>> decision to Sergey and other AWT team members, but I just thought I'd
>> bring this up here.
>>
>> As for the implementation, I see that you're adding realSync() calls
>> in some places where they were not previously there. For example,
>> calling Robot.waitForIdle() before the fix would not cause a
>> realSync() to occur, while after your fix it does. This is a
>> significant change from threading and native event queue
>> synchronization perspectives, and I'm not sure if this should be done.
>> Again, I do know that in tests it is useful to call realSync() here
>> and there, but I'm not sure we should spread the calls all over the
>> Robot implementation simply because of this reason. The calls may in
>> fact produce some unwanted side-effects for existing applications
>> employing the Robot (e.g. introducing unwanted delays, or performing
>> excessive synchronization while the app didn't really need it, etc.) I
>> consider this change very risky.
>>
>> --
>> best regards,
>> Anthony
>>
>> On 8/26/2014 5:40 PM, Dmitriy Ermashov wrote:
>>> Hi awt team!
>>>
>>> A few months ago we have consolidated methods required by functional and
>>> regression tests in ExtendedRobot class. After a period of extensive
>>> testing, it's time to migrate them to java.awt.Robot.
>>>
>>> Please review the changeset:
>>> http://cr.openjdk.java.net/~dermashov/8039749/webrev.00/
>>>
>>> Corresponding bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8039749
>>>
>>> Note that this change is an important prerequisite to a massive
>>> regression and functional test suites change for Jigsaw.
>>> As one can see the webrev contains changes in class signature. The CCC
>>> request will take place after the code review.
>>>
>>> Thanks,
>>> Dima
>