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.






Reply via email to