-
Notifications
You must be signed in to change notification settings - Fork 220
Fix site editor E2E tests for Gutenberg latest #6080
Fix site editor E2E tests for Gutenberg latest #6080
Conversation
update to navigate to site-editor.php and use visitSiteEd...update to navigate to site-editor.php and use visitSiteEditor util once WordPress 6.0 is released. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/43c7c1be26f5582bbbc87fc8d87ba40af86f4b6b/tests/e2e/utils.js#L159-L170🚀 This comment was generated by the automations bot based on a
|
Size Change: 0 B Total Size: 862 kB ℹ️ View Unchanged
|
reading just edited templates states was not stable
update to always use site-editor.php once WordPress 6.0 i...update to always use site-editor.php once WordPress 6.0 is released and verified. Remove usage of GUTENBERG_EDITOR_CONTEXT from from here and from workflows. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/44aeb8a7408eb416b221781e16621f051af867a5/tests/e2e/utils.js#L160-L172🚀 This comment was generated by the automations bot based on a
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing major, code looks excellent, just some small refactoring and you're good to go :)
@@ -167,6 +170,8 @@ describe( 'Store Editing Templates', () => { | |||
await visitTemplateAndAddCustomParagraph( 'single-product' ); | |||
|
|||
await goToSiteEditor( '?postType=wp_template' ); | |||
// we need to wait for the selctor to show up, sometimes the loading is delayed and test becomes falky | |||
await page.waitForSelector( SELECTORS.templates.templateActions ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the interest of separating business logic from testing (like I mentioned in the E2E workshop), it'd be better to move this line within goToSiteEditor
. Basically, it's goToSiteEditor
responsibility to make sure we are in the right place and give back control to the program only when we are (and handle errors when we aren't).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But these wouldn't be there always. It's not a generic check. These would appear only if there were any edited templates available. Should we provide "states" along the path to distinguish what to wait for? We'd probably need another helper ie. goToSiteEditorTemplates(target: 'templates' | 'parts', hasEdits: boolean) or something - maybe then it would make sense.
Should we make it a separate issue to extract these as a separate helpers and think bit more about what utils we actually need there? These page objects would be great for this!
(resolved others for clarity)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe if we were extracting this to a separate hellper util we could use the waitForRespponse
to ensure the templates data was loaded like I did it with the saveTemplate
? Templates data is loaded async after the page was loaded first so we could improve it like so - should make it much more stable and maybe allow us to get rid of the try-catch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, because I know this is a blocker right now for many people, let's merge it. I'd be thankful if you created a new issue so we won't forget about this. Maybe link this thread. 🙏
tests/e2e/utils.js
Outdated
* 1. `themes.php?page=gutenberg-edit-site` is the one used and available if the Gutenberg plugin is enabled. | ||
* 2. `site-editor.php` is the one available in WP Core. | ||
* 1. `themes.php?page=gutenberg-edit-site` this is legacy editor access used for wp <=5.8 compatibility | ||
* themes.php is not accessible in 5.9+ when local Gutenberg is active. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing some backticks for consistency around themes.php
. But, more importantly, I am not sure I understand this comment, could you clarify it a bit?
So themes.php?page=gutenberg-edit-site
seems to be the way to go whenever Gutenberg is installed on <6.0, right? At least that's what I get by reading your code. The comment says themes
is not accessible in 5.9+ when local Gutenberg is active, and I'm not sure I understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Argh, was tweaking text and made error - should be site-editor is not accessible!
Will reword it :)
await page.click( confirmSave ); | ||
await page.waitForSelector( `${ saveButton }[aria-disabled="true"]` ); | ||
await page.waitForResponse( ( res ) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very excellent solution! 👌 +100 for this!
Update to always use site-editor.php once WordPress 6.0 i...Update to always use site-editor.php once WordPress 6.0 is released and fix is verified. Remove usage of GUTENBERG_EDITOR_CONTEXT from from here and from workflows. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/75abdb9db044bc7f4f4e0a5bfb587f7308c9642c/tests/e2e/utils.js#L159-L171🚀 This comment was generated by the automations bot based on a
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved (with reservation 😬 ). It is a blocker for many people so this solution is ok for now. I'm not fond of the repetition and the mixing of business logic and test logic, but we'll take care of it in another issue.
@@ -167,6 +170,8 @@ describe( 'Store Editing Templates', () => { | |||
await visitTemplateAndAddCustomParagraph( 'single-product' ); | |||
|
|||
await goToSiteEditor( '?postType=wp_template' ); | |||
// we need to wait for the selctor to show up, sometimes the loading is delayed and test becomes falky | |||
await page.waitForSelector( SELECTORS.templates.templateActions ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, because I know this is a blocker right now for many people, let's merge it. I'd be thankful if you created a new issue so we won't forget about this. Maybe link this thread. 🙏
This makes the E2E tests on latest Gutenberg to pass again.
GUTENBERG_EDITOR_CONTEXT
env var. It's set on CI to 'gutenberg' to ensure we are usingthemes.php
admin page to interface with templates as because of the E2E tests failing with the last version of Gutenberg #6071 regression it's not possible to use site-editor.php for installed gutenberg and core gutenberg using the same URL. We already hadthemes.php
andsite-editor.php
handling in place but it was not used and had a bug in it. Adding the context now makes use of this and fixed the bug that was hidden there.saveTemplate
util completes by waiting for successful response before handing over control to follow up stepsUpdates wordpress/e2e-test-utils packagedisableSiteEditorWelcomeGuide
TODO for future after WordPress v6 gets released we should review if we can run all the tests just using
site-editor.php
and drop theGUTENBERG_EDITOR_CONTEXT
usage.Fixes #6071
Testing
Automated Tests
Manual Testing
How to test the changes in this Pull Request: