Skip to content

Removed sleep 10s in login method, changed it to loop which waiting field#984

Merged
tomholub merged 17 commits intomasterfrom
tests/issue-825
Nov 12, 2021
Merged

Removed sleep 10s in login method, changed it to loop which waiting field#984
tomholub merged 17 commits intomasterfrom
tests/issue-825

Conversation

@fcvakintos
Copy link
Contributor

@fcvakintos fcvakintos commented Nov 10, 2021

This PR contains changes for login method, removed sleep 10s, added waitForDispalayed({reverse: true})
close #825

Tests

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

this.changeLanguage();
this.gmailLogin(email, password);
this.signInAsGoogleAccounLabel.waitForDisplayed({reverse: true});
this.signInAsGoogleAccounLabel.waitForDisplayed({timeout: 10000, reverse: true});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If various different screens are being switched on the way, you may need to try/wait/retry as I suggested to avoid the flakiness.

@fcvakintos fcvakintos changed the title Removed sleep 10s in login method, changed it to waitForDisplayed Removed sleep 10s in login method, changed it to loop which wait field displayed Nov 11, 2021
@fcvakintos fcvakintos changed the title Removed sleep 10s in login method, changed it to loop which wait field displayed Removed sleep 10s in login method, changed it to loop which waiting field Nov 11, 2021
@fcvakintos
Copy link
Contributor Author

@tomholub finished with this task, run tests and no error (4/4)

do {
browser.pause(1000);
count++;
} while(this.enterPassPhraseField.isDisplayed() !== true || count <= 15);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this does what you think it does. It looks to me it will try 15 times regardless if it showed before or not. Have you tested how many iterations it does?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomholubno, we have "||", it is checking one of arguments is true, if the filed is displayed on 5th iteration, we break from loop, but if the field is not displayed after 15 iterations we finish loop and go to next step
I will check how many iterations appear

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomholub sorry, You was right, I changed || to &&

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomholub checked iterations count
we have max 3 iterations
Screenshot 2021-11-12 at 11 02 20

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks better

@tomholub tomholub enabled auto-merge (squash) November 12, 2021 09:22
@tomholub tomholub merged commit 01235e2 into master Nov 12, 2021
@tomholub tomholub deleted the tests/issue-825 branch November 12, 2021 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

appium: evaluate alternatives to 10s sleep after login

3 participants