E2E: Refactor global inserter utility #46366
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
|
Size Change: -3.37 kB (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
|
@WunderBart, maybe we should extract Block Switcher test fixes into a separate PR. What do you think? Here's my comment regarding the root cause of the issue #45354 (comment). |
Ah, good catch, @Mamaduka! Let's move it to a separate PR. Will make one shortly! 👍 edit: Fix PR: #46406 |
WunderBart
added
[Type] Automated Testing
WunderBart
changed the title
WunderBart
commented
Dec 9, 2022
| @@ -166,7 +166,7 @@ const matchUrl = ( reqUrl, urls ) => { | |||
| }; | |||
|
|
|||
| describe( 'adding blocks from block directory', () => { | |||
| beforeEach( async () => { | |||
| beforeAll( async () => { | |||
| await createNewPost(); | |||
There was a problem hiding this comment.
For some reason the adding blocks from block directory › Should be able to add (the first) block spec started throwing the following error:
[Error: ReferenceError: wp is not defined
at https://fake_url.com/block.js:2:26
at https://fake_url.com/block.js:17:4]
[Error: ReferenceError: wp is not defined
at https://fake_url.com/block.js:2:26
at https://fake_url.com/block.js:17:4]
FAIL packages/e2e-tests/specs/editor/plugins/block-directory-add.test.js (106.681 s)
● adding blocks from block directory › Should be able to add (the first) block.
TimeoutError: waiting for selector `.edit-post-layout` failed: timeout 30000ms exceeded
at new WaitTask (../../node_modules/puppeteer-core/src/common/DOMWorld.ts:813:28)
at DOMWorld.waitForSelectorInPage (../../node_modules/puppeteer-core/src/common/DOMWorld.ts:656:22)
at Object.internalHandler.waitFor (../../node_modules/puppeteer-core/src/common/QueryHandler.ts:78:19)
at DOMWorld.waitForSelector (../../node_modules/puppeteer-core/src/common/DOMWorld.ts:511:25)
at Frame.waitForSelector (../../node_modules/puppeteer-core/src/common/FrameManager.ts:1273:47)
at Page.page [as waitForSelector] (../../node_modules/puppeteer-core/src/common/Page.ts:[32](https://github.com/WordPress/gutenberg/actions/runs/3650930460/jobs/6167542890#step:6:33)10:29)
at createNewPost (../e2e-test-utils/build/@wordpress/e2e-test-utils/src/create-new-post.js:[37](https://github.com/WordPress/gutenberg/actions/runs/3650930460/jobs/6167542890#step:6:38):8)
at Object.<anonymous> (specs/editor/plugins/block-directory-add.test.js:176:3)
I haven't been able to figure out how this is related to my changes as I cannot reproduce locally, but the reason might be the request interception bleeding into the post-new.php request. No idea why it would start happening now. Anyway, letting both steps be tested on the same post fixes the issue, and doesn't degrade the value of the spec so it's OK to go with this fix IMO.
Pinging @StevenDufresne and @TimothyBJacobs (who I believe worked on most part of this spec) for insight on what might be going on here. 🙏
There was a problem hiding this comment.
Sorry, I don't think I can be of much help here. My work was mainly on the REST API side of things.
There was a problem hiding this comment.
Hey, I've noted this. Hopefully, I can take a look before the end of the week although a lot has changed in the Gutenberg project since we landed the original PR.
There was a problem hiding this comment.
Looking at this, it doesn't appear related to that test in specific but instead a timeout upstream. I suggest try re-running the scripts. I was able to make your tests pass.
There was a problem hiding this comment.
Thanks for checking, @StevenDufresne! The tests are passing on this branch though due to this change I had to make. I've probably not made it clear enough in my explanation above – sorry for that. The test should start failing in CI once the 2ee8335 commit is reverted (currently the last one). Should I revert it on this branch so you can investigate it in CI?
There was a problem hiding this comment.
Ah, my mistake.
I think investigating would be investing more time than it's worth. If you want to be 100% safe, view the test run interactively and if it still adds the block to the document we are good. I would be pretty surprised if it passed without doing so though. 👍
There was a problem hiding this comment.
Gotcha. I did check via the interactive mode, and the block is correctly added to the canvas. 🙌
/cc @tyxla, this one should be ready for a review if you have a spare cycle! 🙏
There was a problem hiding this comment.
Sounds good, thanks @WunderBart! I'll take a look tomorrow if I don't get to it today.
tyxla
approved these changes
Dec 16, 2022
There was a problem hiding this comment.
This looks great, @WunderBart, much better than what we had!
I've left a few minor clarification ideas, but other than that this looks stellar 🚀
Thank you 🙌
|
There's one unrelated failure, which is also happening on edit: Passed after a rerun. |
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.
What?
Refactor the Global Inserter E2E utils:
openGlobalInserteris called,searchGlobalInsertermethod,insertFromGlobalInsertermethod,Why?
I'm aware that this might be excessive due to the ongoing Puppeteer -> Playwright migration. Still, the Inserter is one of the most used components in our E2E tests, so it's worth the effort to make the utility a bit more stable and faster until the migration is done.
Testing Instructions
All (inserter-related) E2E tests should be passing.