The fix looks good to me.
Thanks,
Alexandr.
On 23/05/16 22:52, Alexandre (Shura) Iline wrote:
I have updated the web rev:
http://cr.openjdk.java.net/~shurailine/8157339/webrev.01/
<http://cr.openjdk.java.net/%7Eshurailine/8157339/webrev.01/>
I imagine I need an approval from one of the reviewers on the alias.
Sergey, Phil, could any of you take a look or recommend somebody else
for that?
Shura
On May 23, 2016, at 10:48 AM, Alexandre (Shura) Iline
<alexandre.il...@oracle.com <mailto:alexandre.il...@oracle.com>> wrote:
On May 20, 2016, at 3:29 PM, Alexander Kouznetsov
<alexander.kouznet...@oracle.com
<mailto:alexander.kouznet...@oracle.com>> wrote:
Shura,
Great changes.
I have several comments on them:
* Seems like @key intermittent should be added to
ButtonDemoScreenshotTest.
That is an option. I will add this.
* OptionPaneDemoTest.java:
o Looks like the following line should be removed:
88 final String labelText = COMPONENT_R3;
Yes.
o
o And index 1 is not needed in the variable name here:
87 JDialogOperator jdo1 = new JDialogOperator(MESSAGE);
o Shouldn't waitText be used here?
97 if(textToType != null) {
98 new JTextFieldOperator(jdo).typeText(textToType);
99 }
Well, I do believe that this is not necessary for the positive case.
Reason is that the input events are posted, the application would,
sooner or later react on them. If it does not, this would be a bug in
Swing, as, later, we _wait_ for the proper label in a dialog. I could
make some sense for negative case. I will add the waiting.
o
as it is used below here:
191 jto.typeText(TEXT_TO_TYPE);
192 jto.waitText(TEXT_TO_TYPE);
* TreeDemoTest
o Description is not correct here:
+ private void waitRowCount(JTreeOperator tree, int count) {
+ tree.waitState(new ComponentChooser() {
+ public boolean checkComponent(Component comp) {
+ return tree.getRowCount() == count;
+ }
+ public String getDescription() {
+ return "All nodes to be expanded in the tree";
+ }
+ });
+ }
Yes.
o
* I'd propose to introduce a lambda-compliant method to easily
wait for any state without writing many lines of code. Something
like:
waitState("All nodes to be expanded in the tree", () ->
tree.getRowCount() == NODES_TOTAL);
That would allow to eliminate several methods like
waitRowCount() above.
Agree, although this should go into Jemmy. And it could be a further
improvements.
Thank you for the review, Alexander!
Shura
*
Best regards,
Alexander Kouznetsov
(408) 276-0387
On 5/19/2016 8:59 AM, Alexandre (Shura) Iline wrote:
Hi.
Please take a look on the changes
in:http://cr.openjdk.java.net/~shurailine/8157339/webrev.00/
I have gone through the code to discover potential instabilities. I was not
changing the test logic much and mostly changed the code where I had to change
it for stabilization reason. In some cases it was easier to move logic into
methods to avoid copy-pasting it around.
The other bits of work which I was able to identify (TODOs), do not affect test
stability, as far as I can see.
I am CCing Praveen Mohan and Alexander Kouznetsov who had worked on that code
before.
Shura.