-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate Preferences Test to Playwright #44107
Migrate Preferences Test to Playwright #44107
Conversation
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.
Thanks for working on this.
await expect( | ||
page.locator( | ||
'.components-button.edit-post-sidebar__panel-tab.is-active' | ||
) | ||
).toHaveText( 'Post' ); |
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.
The test seems to be failing on this line.
The issue could be that Playwright isn't waiting for the element. You could try using expect.poll
.
It'd be good to try changing the classname selector to a role selector. This is what my browser dev tools shows as the accessibility properties:
(so role is "button", name is "Post (selected)")
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.
Hi @talldan, Previously also tried expect.poll
but expect.poll excepts function only. I also tried to pass the async function within it but still it seems to be not working.
Let me know your thoughts on same.
page.locator( | ||
'.components-button.edit-post-sidebar__panel-tab.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.
page.locator( | ||
'.components-button.edit-post-sidebar__panel-tab.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.
This is the same element as the locator from my first comment (role "button", name "Post (selected)").
It could be assigned to a variable at the start of the test and that variable used here.
|
||
// Dismiss. | ||
await page.click( | ||
'.edit-post-sidebar__panel-tabs [aria-label="Close settings"]' |
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.
// See: https://github.com/WordPress/gutenberg/issues/6377 | ||
// See: https://github.com/WordPress/gutenberg/pull/8995 | ||
await page.reload(); | ||
await page.waitForSelector( '.edit-post-layout' ); |
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.
This line essentially waits for the block editor to load, but I think it line can be removed. The line below it could be changed to await expect.poll(
, which would make the test wait for the panel tab to be present (so it would have the same result of waiting for the editor to load).
'.edit-post-sidebar__panel-tabs [aria-label="Close settings"]' | ||
); | ||
await expect( | ||
page.locator( '.edit-post-sidebar__panel-tab.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.
Again I think it'd be fine to use the locator from my first comment (assign it to a variable and use the variable here).
|
||
// Remember after reload. | ||
await page.reload(); | ||
await page.waitForSelector( '.edit-post-layout' ); |
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.
await page.reload(); | ||
await page.waitForSelector( '.edit-post-layout' ); | ||
await expect( | ||
page.locator( '.edit-post-sidebar__panel-tab.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.
Same again, it'd be fine to use the locator from my first comment (assign it to a variable and use the variable here).
Hi @talldan, Thanks for the feedbacks, I have resolved all of those. Could you please review once? |
Hi @talldan, Any further suggestions on this PR? |
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.
It looks like there's a merge conflict. Can you rebase your branch on top of the latest trunk and resolve the conflict?
Thank you!
What?
Part of #38851.
Migrate preferences.test.js to its Playwright version.
Why?
Part of #38851.
How?
See MIGRATION.md for migration steps.
Testing Instructions
Run
npm run test-e2e:playwright test/e2e/specs/editor/various/preferences.spec.js