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: Use waitUntil instead of waitForLoadState in page.goto() #37831

Merged
merged 4 commits into from Apr 19, 2023

Conversation

ravinderk
Copy link
Contributor

Submission Review Guidelines:

Changes proposed in this Pull Request:

I found that in few playwright tests, we are using waitForLoadState. This is not correct argument to goto and click. I replace it with waitUntil.

Closes #36698 .

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Changed playwright E2E tests should pass.

@github-actions github-actions bot added focus: e2e tests Issues related to e2e tests plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution labels Apr 19, 2023
@woocommercebot woocommercebot requested review from a team and vedanshujain and removed request for a team April 19, 2023 11:19
@ravinderk ravinderk marked this pull request as ready for review April 19, 2023 11:19
@vedanshujain vedanshujain requested review from a team and removed request for vedanshujain April 19, 2023 11:20
@codecov
Copy link

codecov bot commented Apr 19, 2023

Codecov Report

Merging #37831 (b980454) into trunk (e8fcfbb) will decrease coverage by 0.0%.
The diff coverage is n/a.

❗ Current head b980454 differs from pull request most recent head 24e83c2. Consider uploading reports for the commit 24e83c2 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             trunk   #37831     +/-   ##
==========================================
- Coverage     51.5%    51.5%   -0.0%     
- Complexity   17261    17281     +20     
==========================================
  Files          429      430      +1     
  Lines        79957    80030     +73     
==========================================
+ Hits         41215    41217      +2     
- Misses       38742    38813     +71     

see 6 files with indirect coverage changes

@ravinderk
Copy link
Contributor Author

@tammullen I reverted changes related to page.click. This pull request is ready to review.

lanej0
lanej0 previously approved these changes Apr 19, 2023
Copy link
Contributor

@lanej0 lanej0 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! Thank you for this contribution!

Remove waits, failing on CI
@lanej0 lanej0 merged commit 2239270 into woocommerce:trunk Apr 19, 2023
17 of 18 checks passed
@github-actions github-actions bot added this to the 7.8.0 milestone Apr 19, 2023
@nigeljamesstevenson
Copy link
Contributor

Hi folks, a bit late to the party on this one.

I might be missing something here but it appears that the code { waitForLoadState: 'networkidle' } has been removed rather than being replaced with { waitUntil: 'networkidle' } as per the playwright docs

Is this expected @lanej0 @tammullen?

@lanej0
Copy link
Contributor

lanej0 commented Apr 19, 2023

That was me @nigeljamesstevenson -- the tests started failing with the update. I don't think we need an explicit wait in those places anyhow, so I tried removing and ran the tests a bunch of times without issue.

@ravinderk ravinderk deleted the tweak/36698 branch April 20, 2023 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contributor Day - H1 2023 focus: e2e tests Issues related to e2e tests plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E2E: Use waitUntil instead of waitForLoadState in page.goto()
5 participants