Hi Anthony,
On 08/28/2014 12:09 PM, Anthony Petrov wrote:
Hi Dmitriy, Yuri,
On 8/27/2014 4:54 PM, Yuri Nesterenko wrote:
On 08/27/2014 04:25 PM, Dmitriy Ermashov wrote:
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.
Could you please clarify as to why it won't compile? IIUC, all the GUI
tests that require Robot will be in a single module (the java.desktop
one I suppose?), so why wouldn't ExtendedRobot compile while the tests
themselves would if they all are located in the same module?
All jtreg tests are in default package, ExtendedRobot class as well.
When we trying to run any jtreg test that uses ExtendedRobot it fails
with the following error:
java.lang.IllegalAccessError: tried to access class sun.awt.SunToolkit
from class ExtendedRobot
As you can see the problem is not in tests that uses ExtendedRobot, but
in code that uses sun.* and other internal packages. This is the
limitation we are trying to deal with.
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.
Yes, I realize that some changes will be needed for this. My concern
is that having realSync() being called unconditionally from an
existing public method may have an impact on existing applications
that use the Robot for tasks other than testing (e.g. for simple
automation tasks, etc.) And I'm not sure if we're able to estimate all
the potential side effects that this change can bring.
We already have a kind of condition: a variable isAutoWaitForIdle.
waitForIdle method for now is called only from inside other Robot
methods and in case when isAutoWaitForIdle flag is set to true
(fortunately it's default value is false).
Thanks,
Dima
Again, if the tests go into the java.desktop module, then I don't see
a problem with calling realSync() from sun.awt.SunToolkit directly as
you did before. If this is not the case, then I think I'd prefer to
introduce a separate single public method in the Toolkit (or Robot?)
class that would allow applications (and tests) to actually perform
the realSync() operation.
--
best regards,
Anthony
-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
