This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Try to fix E2E tests once and for all #6337
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
rubikuserbot
requested review from
a team and
nielslange
and removed request for
a team
alexflorisca
changed the title
|
Size Change: +689 B (0%) Total Size: 871 kB
ℹ️ View Unchanged
|
senadir
reviewed
Apr 28, 2022
| - name: E2E Tests (WP latest with Gutenberg plugin) | ||
| env: | ||
| WOOCOMMERCE_BLOCKS_PHASE: 3 | ||
| GUTENBERG_EDITOR_CONTEXT: 'gutenberg' | ||
| run: | | ||
| chmod -R 767 ./ #needed for permissions issues | ||
| JSON='{"plugins": ["https://downloads.wordpress.org/plugin/woocommerce.latest-stable.zip","https://github.com/WP-API/Basic-Auth/archive/master.zip","https://downloads.wordpress.org/plugin/gutenberg.latest-stable.zip", "."] }' |
There was a problem hiding this comment.
This line has moved to wp-env-with-gutenberg.js, we still use Basic-Auth I think
There was a problem hiding this comment.
@senadir this was overwriting completely the JSON file, and Alex's changes read the JSON and only push the Gutenberg entry on the "plugins" object, so the rest remains unchanged.
alexflorisca
changed the title
ralucaStan
reviewed
Apr 29, 2022
| @@ -111,6 +105,7 @@ jobs: | |||
| run: | | |||
| npm run wp-env start | |||
| npm run wp-env clean all | |||
| mkdir -p reports/e2e/screenshots | |||
There was a problem hiding this comment.
Does this work on the CI? I expect this to work locally, but I am not aware if something like this is supported on the CI? Would we have screenshots uploaded when a test fails?
There was a problem hiding this comment.
I actually realised @opr already added screenshots to failing tests 2 months ago, so I'll remove this, thanks for reminding me. And yeah, the mkdir command should work in CI, as we are specifying running it on ubuntu, and mkdir is installed by default. There are safer ways to do it though for sure!
ralucaStan
approved these changes
Apr 29, 2022
| @@ -1,22 +1,22 @@ | |||
| name: "Close stale issues" | |||
There was a problem hiding this comment.
There isn’t any relevant change in this file for the tests right?
There was a problem hiding this comment.
Nope this is just linting corections
tests/utils/shopper.js
Outdated
| }, | ||
|
|
||
| goToCheckout: async () => { | ||
| await shopper.block.goToBlockPage( 'Checkout' ); | ||
| // Wait for Checkout block to finish loading |
There was a problem hiding this comment.
I would add extra details here and for the previous utils that it is good practice to wait for the block to load, otherwise we will get flaky tests.
tests/utils/shopper.js
Outdated
| shippingName | ||
| ); | ||
|
|
||
| // eslint throws an errors saying this is aoutside |
There was a problem hiding this comment.
Suggested change
| // eslint throws an errors saying this is aoutside | |
| // eslint throws an errors saying this is outside |
| await expect( page ).toMatchElement( | ||
| '.wc-block-components-totals-shipping .wc-block-formatted-money-amount', | ||
| { | ||
| text: shippingPrice, | ||
| timeout: 5000, | ||
| timeout: 30000, |
There was a problem hiding this comment.
Is this still needed, I would add a small comment if so
wavvves
approved these changes
Apr 29, 2022
opr
added
tools
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
After digging a little deeper, there were some actual issues that were making the tests fail, not just flakiness. Saying that, there are still some of those funky looking errors we all love:
From the jest repo, it looks like something outside of the tests is failing. Didn't have time to dig into this one though.
Also note there might be a few irrelevant github workflows changed, only because of updated linting
Fixes #6286
So what went wrong?
Flakey
switchUserToAdmin().The wordpress/e2e-test-utils function
switchUserToAdmin()is flakey and seems to fail quite often. Instead we should usemerchant.login()which seems a bit more stable. My guess is because of this line. But either way, Gutenberg is migrating to Playwright and those test-utils are not being actively maintained any more so I wouldn't rely on them too much going forward.Missing
woo-test-helperpluginThe woo-test-helper plugin was missing from the E2E tests (with Gutenberg) github action. There are a couple of sneaky lines of code that overrides the plugins entry of the
.wp-env.json. If you don't know they exist, it's very easy to miss when adding a new plugin to the wordpress test env.I created
./bin/wp-env-with-gutenberg.jsscript to extend the existing.wp-env.jsonwith the Gutenberg plugin. This could be extended in the future with other features if needed (passing command line args, etc), but kept it simple for now. The benefit is you only have to define the wp-env config in one place.goToCheckout()andgoToCart()functions were not waiting for the block to loadAS it says on the tin. We were waiting for the page to load, but not for the blocks to finish loading. This was causing flakiness when sometimes the assertions would run after the block was loaded (and pass) and sometimes before (and would fail)
Placing orders without filling in the checkout
There were a couple of instances where we were calling
shopper.block.placeOrder()and the checking for order summary page, without actually filling in the details on the checkout page. This sometimes worked because the tests that run before it may have stored some details in the browser cache, and hence the flakiness. If these tests were to run first, it would almost certainly fail as there would be no details entered.Calling
page.waitForNavigation()andpage.click()The
visitBlockPagewas callingpage.waitForNavigation()afterpage.click(). With this approach, there are some rare cases where the page can navigate immediately after the click and before the waitForNavigation has had a chance to register its event handlers. The recommended approach from the puppeteer docs isUse
networkidle0instead ofdomcontentloadedwhen navigatingThere were a couple of instances that were using
page.waitForNavigation( { waitUntil: 'domcontentloaded' } ). This might work for shortcode based pages, but blocks tend to load more resources async after dom content loaded, so we should use{ waitUntil: 'networkidle0' }for extra safety to make sure all resources have loaded after a page navigation.To Test