-
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 remaining 'publish panel' e2e tests to Playwright #57432
Conversation
Size Change: 0 B Total Size: 1.69 MB ℹ️ View Unchanged
|
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! Just small nitpicks that don't block the PR.
await expect( | ||
page | ||
.getByRole( 'region', { name: 'Editor publish' } ) | ||
.locator( ':focus' ) | ||
).toHaveText( 'Publish' ); |
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.
Could we instead rewrite this into toBeFocused
to better match the description?
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.
Don't have a strong opinion, but the original test was checking the activeElement
.
await expect( | ||
page | ||
.getByRole( 'region', { name: 'Editor publish' } ) | ||
.locator( ':focus' ) | ||
).toHaveText( 'Test 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.
Ditto.
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 here.
What?
Part of #38851.
PR migrates remaining
publish-panel.test.js
e2e tests to Playwright.Why?
See #38851.
Testing Instructions