Hi, Dmitriy.
Do I understand correctly that these methods will be added to the Robot
class in jdk9?
A few notes:
- Different javadoc formatting for different methods.
- There is no information about exceptions. At least waitForIdle will
throw an exception if will be called on edt.
On 31.03.2014 15:51, Dmitriy Ermashov wrote:
Hi,
Please, review the latest version of changeset for:
https://bugs.openjdk.java.net/browse/JDK-8038631
Webrev is here:
http://cr.openjdk.java.net/~yan/8038631/webrev.01/
On 31.03.2014 14:15, Petr Pchelko wrote:
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?
And couldn't you reuse DEFAULT_SPEED for sleeping in mouseClick? The
comment states it's used there but it's not..
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.
Ok, no problem. I'm fine either way.
With best regards. Petr.
On 31.03.2014, at 14:11, Dmitriy Ermashov
<[email protected]> wrote:
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
--
Best regards, Sergey.
