-
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
Add some basic e2e tests for the widgets screen #28160
Conversation
Size Change: +188 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
da4ad6a
to
5b3d1c8
Compare
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.
Nice work! I think we should look at snapshotting the actual widget entities (getWidgets()
) instead of the block representation (getWidgetAreas()
). Other than that, this looks good as something that we can merge and iterate on in follow-up PRs.
} ); | ||
|
||
beforeAll( async () => { | ||
// TODO: Ideally we can bundle our test theme directly in the repo. |
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.
There's some prior art here with the tt1-blocks
theme which is used to test FSE.
Line 6 in d61dccc
"themes": [ "WordPress/theme-experiments/tt1-blocks" ], |
We can look into it later, though. It's more important that we get some tests for widgets in right now 🙂
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.
Yeah, I'm aware of that, but figured we should do it in a following PR 👍 .
await activateTheme( 'twentytwentyone' ); | ||
} ); | ||
|
||
async function getParagraphBlockInGlobalInserter() { |
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.
Nit: Why are some of these helper functions at the top here and some at the bottom of the file? 😀
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's just a pattern more aligned to my mental model 😆 . Functions here are regular utility functions, but functions at the bottom should eventually be moved to the e2e-test-utils
package once they're stable.
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's a great start! Nice work.
// TODO: Probably need a more accessible way to select this, maybe a test ID or data attribute. | ||
const insertionPointIndicator = await page.$( | ||
'.block-editor-block-list__insertion-point-indicator' | ||
); |
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 #28185.
// FIXME: The insertion point indicator is not showing when the widget area has no blocks. | ||
// await expectInsertionPointIndicatorToBeBelowLastBlock( | ||
// firstWidgetArea | ||
// ); |
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 #28123.
Also, unless there are multiple widgets in a sidebar area, I'm unable to get the in-between inserter to appear.
|
||
beforeAll( async () => { | ||
// TODO: Ideally we can bundle our test theme directly in the repo. | ||
await activateTheme( 'twentytwenty' ); |
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.
Why specifically TwentyTwenty?
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 just happens to have 2 widget areas so that we can test DnD and some other stuffs more easily 😅 .
Thanks for the test Should we move it to |
@youknowriad Ahh got it, I didn't know what it means and where to put this, and seems like most of us don't know 😆 : #28160 (comment). |
Description
Close #25784.
An attempt to add some basic e2e tests for the widgets screen following the rules in #27260 but without introducing a new library yet.
The selectors are mostly using aria attributes or human-readable text, to simulate what the actual users see/read. Some exceptions are made to use
data
attribute where there's no stable a11y queries available.You will see a lot of
TODO
s andFIXME
s here 😅. We're planning to fix them in following PRs or open issues.The top priority for this PR is to make sure the tests pass consistently.
How has this been tested?
Types of changes
Bug fix
Checklist: