-
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 'Block variations' e2e tests to Playwright #53266
Conversation
).toBeDefined(); | ||
} ); | ||
|
||
test( 'Insert the Large Quote block variation with slash command', 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.
I removed this test during migration. The previous test case uses the slash command, so we already cover this case.
Size Change: 0 B Total Size: 1.44 MB ℹ️ View Unchanged
|
Flaky tests detected in ce4358b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5737636437
|
@kevin940726, it would be nice to land this refactoring as the test in question fails quite often. |
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.
Sorry for the late review! Very nicely done! Just have a couple of questions but none are blocking 👍 .
await expect( | ||
editor.canvas | ||
.getByRole( 'document', { name: 'Block: Columns' } ) | ||
.getByRole( 'document' ) |
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.
Should we add { name: /Block: Column \([1-4] of 4\)/ }
here?
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 think it's okay to have this broad locator. We just want to check the number of inner blocks.
).toHaveCount( 4 ); | ||
} ); | ||
|
||
// Tests the `useBlockDisplayInformation` hook. |
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.
Do we want to group these tests with describe
as the original code did?
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.
Not sure. They fit well without the grouping as well and require no special setup.
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.
Either works for me 👍
// @see @wordpres/block-editor/src/components/use-block-display-information (`useBlockDisplayInformation` hook). | ||
describe( 'testing block display information with matching variations', () => { | ||
beforeEach( async () => { | ||
await togglePreferencesOption( |
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.
Sorry if I missed it but I didn't see this in the migrated code. Do we not need this anymore? What was it for?
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 breadcrumbs are enabled by default. If any other test disables it, I think it should enable it back after the tests.
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.
Cool 👍 Thanks for the explanation!
Thanks for the review, @kevin940726! |
What?
Part of #38851.
Closes #47057.
PR migrates 'Block variations' e2e tests to Playwright.
Why?
The
Pick the additional variation in the inserted Columns block
test case failed at least 300 times since January. The number is based on flaky test issue edits. See #47057.Testing Instructions