-
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: End 2 end test to InnerBlocks allowed block restrictions #14054
Add: End 2 end test to InnerBlocks allowed block restrictions #14054
Conversation
f7ffd84
to
30c6a49
Compare
30c6a49
to
546b667
Compare
expect( await getAllInserterItemTitles() ).toMatchSnapshot(); | ||
} ); | ||
|
||
it( 'media & text block should restrict allowed blocks', 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 very specific to the Media & Text block. It's fine to start with but I would personally prefer to see this kind of tests inside blocks/media-text.spec.js
file. It's faster to start with such verification but ideally, we use some custom block registered for testing purpose.
} ); | ||
|
||
it( 'no blocks should be allowed inside columns', async () => { | ||
await insertBlock( 'Columns' ); |
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 same applies as above. Those two test cases should be documented with the explanation of why those blocks are used and what characteristics they have.
Example, if we would change the behavior of Columns blocks to allow some blocks we should rather remove this test from the test suite rather than update it.
72bda4a
to
50e9ef4
Compare
Hi @gziolo, |
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.
Awesome work @jorgefilipecosta. I thought you would handle my comments in a separate PR :)
This looks great, I left a couple of minor things to address before merging. Overall, I'm impressed with how e2e testing infrastructure and coverage grows thanks to your numerous contributions. 🥇
BTW, I ✅ this PR, if you feel you need another review when typos are fixed, let me know.
plugins_url( 'inner-blocks-allowed-blocks/index.js', __FILE__ ), | ||
array( | ||
'wp-blocks', | ||
'wp-components', |
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.
wp-components
and wp-hooks
are never used so can be safely removed.
*/ | ||
function enqueue_block_icons_plugin_script() { | ||
wp_enqueue_script( | ||
'gutenberg-test-block-icons', |
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 name was copied over from another file, it should be updated.
); | ||
} | ||
|
||
add_action( 'init', 'enqueue_block_icons_plugin_script' ); |
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 same issue with the function name, needs to be synced with the name of the file/plugin.
openGlobalBlockInserter, | ||
} from '@wordpress/e2e-test-utils'; | ||
|
||
describe( 'Collumns', () => { |
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.
Typo here:
describe( 'Collumns', () => { | |
describe( 'Columns', () => { |
and in the name of the file.
@@ -15,6 +15,7 @@ export { enablePrePublishChecks } from './enable-pre-publish-checks'; | |||
export { ensureSidebarOpened } from './ensure-sidebar-opened'; | |||
export { findSidebarPanelToggleButtonWithTitle } from './find-sidebar-panel-toggle-button-with-title'; | |||
export { findSidebarPanelWithTitle } from './find-sidebar-panel-with-title'; | |||
export { getAllBlockInserterItemTitles } from './get-all-block-inserter-item-titles'; |
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.
Can we also update CHANGELOG file? This package is public and is already being explored by external projects. This is going to be minor version bump as we add new features 🎉
50e9ef4
to
65dea0c
Compare
Thank you for the feedback @gziolo, it was applied before the merge. |
I received an intermittent failure from these tests in #14128
|
I note #14450 has a failed build as well due to the same error. |
Specifically, the error occurs here in Puppeteer. https://github.com/GoogleChrome/puppeteer/blob/v1.6.2/lib/ElementHandle.js#L60 Also worth noting the file doesn't exist in more recent versions of Puppeteer (unsure if that may have an impact). |
Description
This PR adds an end to end tests to the global inserter.
It verifies that:
Catches the bug being addressed in #14020. So currently the tests fail.