-
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 'embedding' e2e tests to Playwright #57969
Conversation
// Reason: A possible regression of https://github.com/WordPress/gutenberg/pull/14705. | ||
test.skip( 'should retry embeds that could not be embedded with trailing slashes, without the trailing slashes', async ( { |
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 test is currently failing. When I try to test this scenario manually, it doesn't work at all. I'm skipping the test for now, and we can enable it when regression is fixed.
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.
I also confirmed that retry doesn't work. Should we open a new issue as a regression?
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.
Yes, let's open the new issue and reference 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.
Submitted: #58002
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.
Thank you, @t-hamano 🙏
Size Change: 0 B Total Size: 1.7 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.
LGTM! I tested in debug mode and confirmed that all tests are working as expected.
await expect( | ||
currenEmbedBlock.locator( 'figure' ), | ||
'Video content. Should render valid figure element, and include the aspect ratio class.' | ||
).toHaveClass( /wp-embed-aspect-16-9/ ); |
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.
If we strictly migrate the original tests, do we also need to check the .wp-block-embed
and .is-type-video
classes?
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.
I've altered the assertion based on the error message.
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.
I see, that's certainly true 👍
await embedUtils.insertEmbed( 'https://twitter.com/notnownikki' ); | ||
await expect( | ||
currenEmbedBlock.locator( 'iframe' ), | ||
'Valid embed. Should render valid element.' |
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.
I didn't know that we could specify an optional message as the second argument 😄
What?
Part of #38851.
PR migrates
embedding.test.js
e2e tests to Playwright.Why?
See #38851.
Testing Instructions