Migrate multi-currency e2e tests to Playwright #9175
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: 0 B Total Size: 1.32 MB ℹ️ View Unchanged
|
reykjalin
requested changes
Aug 15, 2024
There was a problem hiding this comment.
Love to see the tests being migrated like this and it's an excellent indicator that the tests are passing in GH actions!
However, I ran npm run test:e2e-pw multi-currency 4 times and it failed 3 out of the 4 times. The zip file of the results is in the team channel in Slack. 2 of the failures were identical, the last failure only failed one of the tests, but in the same way as the first 2 failures:
4 failed
[merchant] › merchant/multi-currency.spec.ts:38:6 › Multi-currency › page load without any errors
[shopper] › shopper/multi-currency-checkout.spec.ts:45:8 › Multi-currency checkout › Checkout with multiple currencies › checkout with EUR
[shopper] › shopper/multi-currency-checkout.spec.ts:52:8 › Multi-currency checkout › Checkout with multiple currencies › should display EUR in the order received page
[shopper] › shopper/multi-currency-checkout.spec.ts:60:8 › Multi-currency checkout › Checkout with multiple currencies › should display EUR in the customer order page
The single failure:
1 failed
[merchant] › merchant/multi-currency.spec.ts:38:6 › Multi-currency › page load without any errors
I got similar results when I ran npm run test:e2e-pw merchant/multi-currency and npm run test:e2e-pw shopper/multi-currency; same tests failed in the same places when they did not succeed. They failed about half the time to 2/3rds of the time.
Since it's the same tests failing every time it probably has something to do with the tests themselves rather than anything else?
It's also possible this has something to do with my environment. I just set up the e2e testing for this PR so maybe I did something wrong, or didn't do something correctly 🤔
|
@reykjalin, as I mentioned on Slack, I wasn't able to reproduce the errors from your report, instead I was consistently running into a different error: the order received one. I've made some changes to make the tests more stable. I've run |
cesarcosta99
requested review from
a team and
ricardo
and removed request for
a team
|
Just for reference, this is the Slack thread we have been discussing the latest changes: p1723693966218459-slack-C01BZUL57SQ |
ricardo
reviewed
Aug 20, 2024
There was a problem hiding this comment.
@cesarcosta99 Running the tests locally, the only test that fails consistently for me is should display the correct currency in the my account order history table at shopper/multi-currency-checkout.spec.ts:84:7:
Locator: locator('tr').filter({ has: getByLabel('View order number 142') })
Expected pattern: /USD/
Received string: ""
The test appears to pass in the CI:
1 skipped
34 passed
ricardo
approved these changes
Aug 20, 2024
There was a problem hiding this comment.
Locator: locator('tr').filter({ has: getByLabel('View order number 142') })
Expected pattern: /USD/
Received string: ""
I figured why I was experiencing the error above. I had installed the test environment from scratch but I guess I hadn't removed the cached dependencies, so my WooCommerce version was 8.8.3 still.
Updating it made the test work as the selector was introduced somewhere in between in WooCommerce.
All tests are working consistently for me now. ✅
reykjalin
approved these changes
Aug 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #8750.
Changes proposed in this Pull Request
This PR migrates all multi-currency specs we have in Puppeteer to Playwright:
merchant-admin-multi-currency.spec.js→multi-currency.spec.tsmerchant-admin-multi-currency-on-boarding.spec.js→multi-currency-on-boarding.spec.tsmerchant-admin-multi-currency-setup.spec.js→multi-currency-setup.spec.tsshopper-checkout-multi-currency.spec.js→multi-currency-checkout.spec.tsSome background on decisions I've made while migrating the specs:
merchant-admin/shopperprefix in the filenames because they are within folders with the same name and it sounds redundant.Testing instructions
Setup
npm run test:e2e-setup.Ensure all GH actions below are passing, particularly the E2E Playwright Tests.
Test: Ensure all tests pass locally consistently
You can run the e2e tests multiple times and try different combinations to confirm they all pass and are stable. Some suggestions:
npm run test:e2e-pw multi-currencynpm run test:e2e-pw shopper/multi-currencynpm run test:e2e-pw merchant/multi-currencynpm run test:e2e-pw <multi-currency-spec-name>Test: Ensure the migrated specs test what they're expected to test
You can ensure that by either comparing the old spec code with the new one, and/or watching the tests running via the UI command:
npm run test:e2e-pw-ui multi-currency.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge