I have updated the web rev: http://cr.openjdk.java.net/~shurailine/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> 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: >> Looks like the following line should be removed: >> 88 final String labelText = COMPONENT_R3; > Yes. >> >> And index 1 is not needed in the variable name here: >> 87 JDialogOperator jdo1 = new JDialogOperator(MESSAGE); >> >> 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. >> >> as it is used below here: >> 191 jto.typeText(TEXT_TO_TYPE); >> 192 jto.waitText(TEXT_TO_TYPE); >> TreeDemoTest >> 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. >> >> 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/ >>> <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. >>> >>> >> >
