Template part e2e fix: try and force waiting for button xpath #33169
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: 0 B Total Size: 1.05 MB ℹ️ View Unchanged
|
|
Hi, @getdave How can I help to get this test fixed? I've seen this a few more a lot in the last couple of days. |
|
This might be a good candidate for using |
|
@Mamaduka So what I think is happening is that when the placeholder for Template Part first loads the This process means the selector required to click that button changes. I wonder whether because we're selecting the button by XPath Pupeteer has some internal resolution system which caches the selector as This is largely hypothetical and there could be a simpler explanation. Another option might be to try using a hardcoded testID on the target button and see if that improves the reliability fo the test in the short term with a view to returning and using a more resilient selector in the future. This is a hard one because I cannot replicate locally and even on CI it's sometimes passing. |
getdave
commented
Jul 8, 2021
| @@ -100,10 +100,13 @@ describe( 'Multi-entity save flow', () => { | |||
|
|
|||
| // Add a template part and edit it. | |||
| await insertBlock( 'Template Part' ); | |||
|
|
|||
| await page.waitForXPath( createNewButtonSelector ); | |||
| const [ createNewButton ] = await page.$x( | |||
| createNewButtonSelector | |||
There was a problem hiding this comment.
Let's try updating this selector to use a hardcoded test id or something. If that improves stability then we know my theory is might be correct.
|
@getdave, you're right. The Puppeteer might be holding a different kind of reference internally for that button. Here's a screencast simulating button switching via Network throttling: CleanShot.2021-07-08.at.14.04.52.mp4 |
|
Yes that's the only thing I can think...currently occupied with debugging perf tests for 11.0.0 but will look after that is resolved. Feel free to dig in or take over though 😄 |
|
From the screencast above, I think the reason is because of layout shifting. So the way puppeteer's
All of the above are asynchronous and independent, so if layout shifting is happening between 2 and 3, it will click on the wrong element. I think the solution isn't to solve it in test, but rather to fix it in our code. Whatever puppeteer is experiencing is likely to happen on users too. A solution would be to delay showing the buttons until all requests are resolved, eliminating the intermediate result. Instead, just show a spinner when it's loading, just like how other blocks work. I'm not familiar with template parts blocks though, so no idea if there're any other aspects that I'm not aware of. |
|
Aha. Thanks for that knowledge. I had thought the click was triggered programmatically not like a user click. This is solvable now as you suggest. |
This was referenced Jul 12, 2021
|
Fix made elsewhere based on findings here. Closing. |
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.
Description
This e2e test keeps failing
Looking at the failure artifacts it looks like we've clicked the wrong button and activated the "Choose existing" button.
This is because
*[data-type="core/template-part"].block-editor-block-list__layoutwill only exist once we've click theNew template partbutton.The test selects the "New template part" button by xpath and that resolves to a DOM node with class variant
primary. However the button variant is set to change totertiaryas soon as the "Choose existing" button is rendered. I wonder if this causes the test to click the wrong button?This PR tries to force waiting for the xpath before clicking. We'll see...
How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).