Add end-to-end tests for Navigation block#18929
Conversation
8fb7855 to
2df0378
Compare
This reverts commit 0dd1a36.
0e0bb4e to
3db847b
Compare
31f7d9a to
2216684
Compare
2216684 to
e70e35d
Compare
tellthemachines
left a comment
There was a problem hiding this comment.
Looks good! Left a couple minor comments/questions.
| keyCode: isMac ? 93 : 17, | ||
| which: isMac ? 93 : 17, | ||
| } ), | ||
| } ) |
There was a problem hiding this comment.
Was this an auto lint change? I thought we used trailing commas in our JS.
There was a problem hiding this comment.
Only for arrays/objects, but this is a function parameter list. It wasn't a lint fix, but thought I'd tidy it while I was looking at it.
| // Ideally this would be `await pressKeyWithModifier( 'primary', 'a' )` | ||
| // to select all text like other tests do. | ||
| // Unfortunately, these tests don't seem to pass on Travis CI when | ||
| // using that approach, while using `Home` and `End` they do pass. |
There was a problem hiding this comment.
That's odd. What did the failure look like?
There was a problem hiding this comment.
Yeah, really strange. The call to the function didn't have any effect so the snapshots were failing—where I expected the new text to replace the existing text, instead it was typed into the middle of the field:
"Contact Us" became "ContGet In Touchct Us", but was expected to be "Get In Touch".
Changing the label seems like an important part of the block, so I didn't want to exclude it from the test.
There was a problem hiding this comment.
Again, worked fine locally, just travis that had the difficulties. 🤷♂
There was a problem hiding this comment.
I wonder if the keyboard travis uses is different from what we expect. The bit that's failing is all about simulating keypresses.
| // Create an empty nav block. The 'create' button is disabled until pages are loaded, | ||
| // so we must wait for it to become not-disabled. | ||
| await page.waitForXPath( '//button[text()="Create from all top pages"][not(@disabled)]' ); | ||
| const [ createFromExistingButton ] = await page.$x( '//button[text()="Create from all top pages"][not(@disabled)]' ); |
There was a problem hiding this comment.
If we already awaited for that selector to become available above, do we need to await again?
There was a problem hiding this comment.
The first function waits for the selector to be present. The next one selects the element so that it can be clicked, that part has to be awaited as well, just how the puppeteer api works.
| await createFromExistingButton.click(); | ||
|
|
||
| // Snapshot should contain the mocked pages. | ||
| expect( await getEditedPostContent() ).toMatchSnapshot(); |
There was a problem hiding this comment.
Just curious, did you choose to use snapshots here because the nav item URLs are not shown in the page markup?
There was a problem hiding this comment.
The snapshots mean the test can check all the relevant attributes are being set correctly. The main shortcoming is that for dynamic blocks it doesn't actually check the markup in the post.
| // Add a link to the default Navigation Link block. | ||
| await updateActiveNavigationLink( { url: 'https://wordpress.org', label: 'WP' } ); | ||
|
|
||
| // Move the mouse to reveal the block movers. Without this the test seems to fail. |
There was a problem hiding this comment.
That's weird, the element is definitely visible on the page and it's focusable too, wonder why page.click doesn't work 🤔
There was a problem hiding this comment.
Yep, there were a few curiosities with these tests 😄
It looks like there's a chance the movers will be moved to the toolbar (#18685), so hopefully this won't be a problem for too much longer!
|
Thanks for reviewing @tellthemachines! |
|
This is causing failing e2e tests in |
Description
Closes #18451
Adds e2e tests for the Navigation Block.
How has this been tested?
It is tests.
Types of changes
Tests
Checklist: