Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky e2e login step #4034

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Fix flaky e2e login step #4034

wants to merge 7 commits into from

Conversation

annemirasol
Copy link
Contributor

@annemirasol annemirasol commented Mar 10, 2025

Changes proposed in this Pull Request:

We've been getting a lot of login-related failures for our e2e tests, due to timeouts from waiting for networkidle. Playwright officially discourages the use of networkidle in waitForLoadState (https://playwright.dev/docs/api/class-page#page-wait-for-load-state)

'networkidle' - DISCOURAGED wait until there are no network connections for at least 500 ms. Don't use this method for testing, rely on web assertions to assess readiness instead.

This PR tweaks our login step to wait for load, and use web-first assertions instead of waiting for networkidle.

Testing instructions

Check that e2e tests are green.


  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Changelog entry

  • This Pull Request does not require a changelog entry. (Comment required below)
Changelog Entry Comment

Comment

Post merge

@annemirasol annemirasol marked this pull request as ready for review March 10, 2025 18:45
@annemirasol annemirasol self-assigned this Mar 10, 2025
@annemirasol annemirasol requested review from a team and diegocurbelo and removed request for a team March 10, 2025 18:55
Copy link
Member

@diegocurbelo diegocurbelo left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

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.

2 participants