4. The default value for realSync additional delay doesn't seem
enough. We normally use 500 ms as the frame creation and first
draw could take more time.
We have to use small delay between mousePress and mouseRelease
events to emulate real mouse click event. And larger delay can be
put after mouseRelease call, for ex. 500 ms, as you wrote.
Don't you mind of this solution?
On 31.03.2014 13:12, Petr Pchelko wrote:
Hello, Dmitry.
Some comments:
1. The name RobotWrapper does not fit in my opinion. The
"Wrapper" is more useful for delegation not extension and it
feels like you should pass the normal Robot to the wrapper
constructor.. I think something like "ExtendedRobot" would feel
better. This is my personal preference so it's arguable.
No objections. Will rename it to ExtendedRobot.
2. I'm not sure I understand why is this location of the util
works? in JavaDoc you state that it should be included from
regtesthelpers but you put it to the test lib. Is it some feature
of jtreg I'm not aware of?
Will fix in next version of webrev.
3. Could you please use @code instead of <code>
Will fix in next version of webrev.
4. The default value for realSync additional delay doesn't seem
enough. We normally use 500 ms as the frame creation and first
draw could take more time.
We have to use small delay between mousePress and mouseRelease
events to emulate real mouse click event. And larger delay can be
put after mouseRelease call, for ex. 500 ms, as you wrote.
Don't you mind of this solution?
5. The click method could be more consistent with others if
called "mouseClick". No strong opinion on this.
For now we have several hundreds of functional tests, most of them
use method named click(). It seems more reasonable to create
method click() and use it than search the usage of it in all tests
and fix to mouseClick() call.
6. "Clicks mouse button 1 with the default pause between Press
and Release." this statement does not seem clear. How to override
the default value? Why does this method click with the default
pause and the previous one does not? or does it? It's not clear
from the JavaDoc
I believe, javadoc should be fixed.
Will fix in next version of webrev.
7. Could you please add @Override annotations where appropriate
(waitForIdle for example)
Will fix in next version of webrev.
8. "public void type" may be keyType would be better? For
consistency with other methods.
Same as 5.
9. Could the method you are using for converting KeyChar to
KeyCode be replaces with KeyEvent.getExtendedKeyCodeForChar? Or
KeyStroke API? This switch is ugly and it would be awesome to
replace it.
Will fix in next version of webrev.
Thank you.
With best regards. Petr.
On 31.03.2014, at 12:40, Dmitriy Ermashov
<[email protected]> wrote:
Hi,
Please, review the changeset for:
https://bugs.openjdk.java.net/browse/JDK-8038631
Webrev is here:
http://cr.openjdk.java.net/~yan/8038631/webrev.00
Presently for testing of JDK client we use test suites of two
kinds, historically called regression and functional (internal).
In JDK9 we plan an attempt to unify them and ultimately get rid
of the functional suites.
One of the first technical problems in this refactoring attempt
is a multitude of the java.awt.Robot wrappers. There are some
really elaborate libraries enhansing Robot which all we cannot
and should not port to jtreg.
Fortunately, test writers almost never use complex features of
these wrappers. So here's our plan:
(1) describe the real practice of the robot use in the
functional tests (please don't worry, that's out of scope of
this request);
(2) write a minimal useful RobotWrapper extending java.awt.Robot;
(3) start functional tests refactoring;
(4) cautiously move enhancements to the java.awt.Robot trying
not to break backward compatibility of thousands existing tests.
For instance, waitForIdle should use realSync() but there
definitely
are plenty of tests using it on EDT: or maybe not -- we should
spent some time for background research in parallel with (3).
--
Thanks,
Dima
--
Thanks,
Dima
