-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
chore: make noWaitAfter a default #34608
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This changes the last actions, namely `click` and `press`, to not wait for the ongoing navigations after the action. Maintaining this behavior becomes tricky, because browsers move away from guaranteed synchronous navigations to optimized performance. See microsoft#34377 for more details. This is technically a breaking change. Most of the time, this should not be noticeable, because the next action will auto-wait the navigation and for any required conditions anyway. However, there are some patterns revealed by our tests that are affected: - Calling `goBack/goForward` immediately after an action. This pattern requires `expect(page).toHaveURL()` or a similar check inbetween. - Listening for network events during the action, and immediately asserting after the action. This pattern requires `waitForRequest()` or a similar promise-based waiter as recommended in best practices. We maintain the opt-out `env.PLAYWRIGHT_WAIT_AFTER_CLICK` that reverts to the old behavior for now. Additionally, previous opt-out option `env.PLAYWRIGHT_SKIP_NAVIGATION_CHECK` has been removed, because there have been just a single issue with it, that was immediately addressed in a patch release.
01a89a4
to
af40eae
Compare
Test results for "tests others"21586 passed, 505 skipped Merge workflow run. |
Test results for "tests 1"21 flaky37747 passed, 655 skipped Merge workflow run. |
Test results for "tests 2"1 fatal errors, not part of any test 124 flaky250515 passed, 9729 skipped Merge workflow run. |
This changes the last actions, namely
click
andpress
, to not wait for the ongoing navigations after the action.Maintaining this behavior becomes tricky, because browsers move away from guaranteed synchronous navigations to optimized performance. See #34377 for more details.
This is technically a breaking change. Most of the time, this should not be noticeable, because the next action will auto-wait the navigation and for any required conditions anyway. However, there are some patterns revealed by our tests that are affected:
goBack/goForward
immediately after an action. This pattern requiresexpect(page).toHaveURL()
or a similar check inbetween.waitForRequest()
or a similar promise-based waiter as recommended in best practices.We maintain the opt-out
env.PLAYWRIGHT_WAIT_AFTER_CLICK
that reverts to the old behavior for now.Additionally, previous opt-out option
env.PLAYWRIGHT_SKIP_NAVIGATION_CHECK
has been removed, because there have been just a single issue with it, that was immediately addressed in a patch release.