Skip to content

Conversation

rrrooommmaaa
Copy link
Contributor

@rrrooommmaaa rrrooommmaaa commented Mar 24, 2023

This PR adjusts timeouts

close #5035
close #5038

Tests (delete all except exactly one):

  • Internal test changes

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

@rrrooommmaaa rrrooommmaaa requested a review from sosnovsky as a code owner March 24, 2023 07:36
@rrrooommmaaa rrrooommmaaa enabled auto-merge (squash) March 24, 2023 07:37
@rrrooommmaaa rrrooommmaaa disabled auto-merge March 24, 2023 08:07
@rrrooommmaaa rrrooommmaaa marked this pull request as draft March 24, 2023 08:08
@rrrooommmaaa rrrooommmaaa changed the title don't wait for networkidle2 in live tests Better timeout management in live tests Mar 24, 2023
@rrrooommmaaa rrrooommmaaa marked this pull request as ready for review March 24, 2023 13:06
@rrrooommmaaa rrrooommmaaa enabled auto-merge (squash) March 24, 2023 13:07
Copy link
Collaborator

@sosnovsky sosnovsky left a comment

Choose a reason for hiding this comment

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

Looks good, but mail.google.com/chat test still fails

By the way, why in some cases we use waitUntil: 'load' and in other cases waitUntil: 'networkidle2'?

@rrrooommmaaa rrrooommmaaa merged commit 1eac042 into master Mar 24, 2023
@rrrooommmaaa rrrooommmaaa deleted the issue-5035-waituntil-fix-live-test branch March 24, 2023 15:12
@rrrooommmaaa
Copy link
Contributor Author

By the way, why in some cases we use waitUntil: 'load' and in other cases waitUntil: 'networkidle2'?

load is the default value and it's understandable -- the page has finished loading.
Using networkidle2 was an idea of @limonte, if I'm not mistaken. I don't know the reasoning (to save a couple of seconds?) It works both ways. I left networkidle2 in places where it was originally specified.

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.

Consider increasing TIMEOUT_EACH_RETRY or split the unstable test into several smaller tests Fix live tests

2 participants