-
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
Block Bindings: Refactor e2e tests #65526
Block Bindings: Refactor e2e tests #65526
Conversation
I'm still working on this but I have a few questions so far:
|
Size Change: +510 B (+0.03%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
104ab94
to
c2ce926
Compare
7fa7dd2
to
74b3200
Compare
test( 'should show the key of the custom field in post meta', async ( { | ||
editor, | ||
} ) => { | ||
await editor.insertBlock( { | ||
name: 'core/paragraph', | ||
attributes: { | ||
content: 'paragraph default content', | ||
metadata: { | ||
bindings: { | ||
content: { | ||
source: 'core/post-meta', | ||
args: { key: 'text_custom_field' }, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
const paragraphBlock = editor.canvas.getByRole( 'document', { | ||
name: 'Block: Paragraph', | ||
} ); | ||
await expect( paragraphBlock ).toHaveText( | ||
'text_custom_field' | ||
); | ||
} ); |
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.
test( 'should always show the source label in server-only sources', async ( { | ||
editor, | ||
} ) => { | ||
await editor.insertBlock( { | ||
name: 'core/paragraph', | ||
attributes: { | ||
content: 'paragraph default content', | ||
metadata: { | ||
bindings: { | ||
content: { | ||
source: 'core/server-source', | ||
args: { key: 'text_custom_field' }, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
const paragraphBlock = editor.canvas.getByRole( 'document', { | ||
name: 'Block: Paragraph', | ||
} ); | ||
await expect( paragraphBlock ).toHaveText( 'Server Source' ); | ||
} ); |
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.
test( 'should lock the appropriate controls with a registered source', async ( { | ||
editor, | ||
page, | ||
} ) => { | ||
await editor.insertBlock( { | ||
name: 'core/paragraph', | ||
attributes: { | ||
content: 'paragraph default content', | ||
metadata: { | ||
bindings: { | ||
content: { | ||
source: 'core/post-meta', | ||
args: { key: 'text_custom_field' }, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
const paragraphBlock = editor.canvas.getByRole( 'document', { | ||
name: 'Block: Paragraph', | ||
} ); | ||
await paragraphBlock.click(); | ||
|
||
// Alignment controls exist. | ||
await expect( | ||
page | ||
.getByRole( 'toolbar', { name: 'Block tools' } ) | ||
.getByRole( 'button', { name: 'Align text' } ) | ||
).toBeVisible(); | ||
|
||
// Format controls don't exist. | ||
await expect( | ||
page | ||
.getByRole( 'toolbar', { name: 'Block tools' } ) | ||
.getByRole( 'button', { | ||
name: 'Bold', | ||
} ) | ||
).toBeHidden(); | ||
|
||
// Paragraph is not editable. | ||
await expect( paragraphBlock ).toHaveAttribute( | ||
'contenteditable', | ||
'false' | ||
); | ||
} ); |
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 has been divided in two:
- Check that post meta is locked in templates: https://github.com/WordPress/gutenberg/pull/65526/files#diff-c01638bdd27a3ad3d6cb09aacb818bc1d0ad24b32065b19631abdcd7fe22ebedR35-R61
- Check all the controls are locked when paragraph is not editable: https://github.com/WordPress/gutenberg/pull/65526/files#diff-70af8def41b67ce618e94f6b4f68044bb4540816a1a8084a1ba1eac9e0c26595R265-R310
test( 'should lock the appropriate controls when source is not defined', async ( { | ||
editor, | ||
page, | ||
} ) => { | ||
await editor.insertBlock( { | ||
name: 'core/paragraph', | ||
attributes: { | ||
content: 'paragraph default content', | ||
metadata: { | ||
bindings: { | ||
content: { | ||
source: 'plugin/undefined-source', | ||
args: { key: 'text_custom_field' }, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
const paragraphBlock = editor.canvas.getByRole( 'document', { | ||
name: 'Block: Paragraph', | ||
} ); | ||
await paragraphBlock.click(); | ||
|
||
// Alignment controls exist. | ||
await expect( | ||
page | ||
.getByRole( 'toolbar', { name: 'Block tools' } ) | ||
.getByRole( 'button', { name: 'Align text' } ) | ||
).toBeVisible(); | ||
|
||
// Format controls don't exist. | ||
await expect( | ||
page | ||
.getByRole( 'toolbar', { name: 'Block tools' } ) | ||
.getByRole( 'button', { | ||
name: 'Bold', | ||
} ) | ||
).toBeHidden(); | ||
|
||
// Paragraph is not editable. | ||
await expect( paragraphBlock ).toHaveAttribute( | ||
'contenteditable', | ||
'false' | ||
); | ||
} ); | ||
} ); |
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.
test( 'should show the key of the custom field', async ( { | ||
editor, | ||
} ) => { | ||
await editor.insertBlock( { | ||
name: 'core/heading', | ||
attributes: { | ||
content: 'heading default content', | ||
metadata: { | ||
bindings: { | ||
content: { | ||
source: 'core/post-meta', | ||
args: { key: 'text_custom_field' }, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
const headingBlock = editor.canvas.getByRole( 'document', { | ||
name: 'Block: Heading', | ||
} ); | ||
await expect( headingBlock ).toHaveText( 'text_custom_field' ); | ||
} ); |
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.
test( 'should be possible to edit the value of the url custom field from the button', async ( { | ||
editor, | ||
page, | ||
pageUtils, | ||
} ) => { | ||
await editor.insertBlock( { | ||
name: 'core/buttons', | ||
innerBlocks: [ | ||
{ | ||
name: 'core/button', | ||
attributes: { | ||
anchor: 'button-url-binding', | ||
text: 'button default text', | ||
url: '#default-url', | ||
metadata: { | ||
bindings: { | ||
url: { | ||
source: 'core/post-meta', | ||
args: { key: 'url_custom_field' }, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
], | ||
} ); | ||
|
||
// Edit the url. | ||
const buttonBlock = editor.canvas | ||
.getByRole( 'document', { | ||
name: 'Block: Button', | ||
exact: true, | ||
} ) | ||
.getByRole( 'textbox' ); | ||
await buttonBlock.click(); | ||
await page | ||
.getByRole( 'button', { name: 'Edit link', exact: true } ) | ||
.click(); | ||
await page | ||
.getByPlaceholder( 'Search or type URL' ) | ||
.fill( '#url-custom-field-modified' ); | ||
await pageUtils.pressKeys( 'Enter' ); | ||
|
||
// Check that the button url attribute didn't change. | ||
const [ buttonsObject ] = await editor.getBlocks(); | ||
expect( buttonsObject.innerBlocks[ 0 ].attributes.url ).toBe( | ||
'#default-url' | ||
); | ||
// Check the value of the custom field is being updated by visiting the frontend. | ||
const previewPage = await editor.openPreviewPage(); | ||
await expect( | ||
previewPage.locator( '#button-url-binding a' ) | ||
).toHaveAttribute( 'href', '#url-custom-field-modified' ); | ||
} ); |
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.
test( 'should be possible to edit the value of the url custom field from the image', async ( { | ||
editor, | ||
page, | ||
pageUtils, | ||
requestUtils, | ||
} ) => { | ||
const customFieldMedia = await requestUtils.uploadMedia( | ||
path.join( | ||
'./test/e2e/assets', | ||
'1024x768_e2e_test_image_size.jpeg' | ||
) | ||
); | ||
imageCustomFieldSrc = customFieldMedia.source_url; | ||
|
||
await editor.insertBlock( { | ||
name: 'core/image', | ||
attributes: { | ||
anchor: 'image-url-binding', | ||
url: imagePlaceholderSrc, | ||
alt: 'default alt value', | ||
title: 'default title value', | ||
metadata: { | ||
bindings: { | ||
url: { | ||
source: 'core/post-meta', | ||
args: { key: 'url_custom_field' }, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
|
||
// Edit image url. | ||
await page | ||
.getByRole( 'toolbar', { name: 'Block tools' } ) | ||
.getByRole( 'button', { | ||
name: 'Replace', | ||
} ) | ||
.click(); | ||
await page | ||
.getByRole( 'button', { name: 'Edit link', exact: true } ) | ||
.click(); | ||
await page | ||
.getByPlaceholder( 'Search or type URL' ) | ||
.fill( imageCustomFieldSrc ); | ||
await pageUtils.pressKeys( 'Enter' ); | ||
|
||
// Check that the image url attribute didn't change and still uses the placeholder. | ||
const [ imageBlockObject ] = await editor.getBlocks(); | ||
expect( imageBlockObject.attributes.url ).toBe( | ||
imagePlaceholderSrc | ||
); | ||
|
||
// Check the value of the custom field is being updated by visiting the frontend. | ||
const previewPage = await editor.openPreviewPage(); | ||
await expect( | ||
previewPage.locator( '#image-url-binding img' ) | ||
).toHaveAttribute( 'src', imageCustomFieldSrc ); | ||
} ); |
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.
test( 'should be possible to edit the value of the text custom field from the image alt', async ( { | ||
editor, | ||
page, | ||
} ) => { | ||
await editor.insertBlock( { | ||
name: 'core/image', | ||
attributes: { | ||
anchor: 'image-alt-binding', | ||
url: imagePlaceholderSrc, | ||
alt: 'default alt value', | ||
metadata: { | ||
bindings: { | ||
alt: { | ||
source: 'core/post-meta', | ||
args: { key: 'text_custom_field' }, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
const imageBlockImg = editor.canvas | ||
.getByRole( 'document', { | ||
name: 'Block: Image', | ||
} ) | ||
.locator( 'img' ); | ||
await imageBlockImg.click(); | ||
|
||
// Edit the custom field value in the alt textarea. | ||
const altInputArea = page | ||
.getByRole( 'tabpanel', { name: 'Settings' } ) | ||
.getByLabel( 'Alternative text' ); | ||
await expect( altInputArea ).not.toHaveAttribute( 'readonly' ); | ||
await altInputArea.fill( 'new value' ); | ||
|
||
// Check that the image alt attribute didn't change. | ||
const [ imageBlockObject ] = await editor.getBlocks(); | ||
expect( imageBlockObject.attributes.alt ).toBe( | ||
'default alt value' | ||
); | ||
// Check the value of the custom field is being updated by visiting the frontend. | ||
const previewPage = await editor.openPreviewPage(); | ||
await expect( | ||
previewPage.locator( '#image-alt-binding img' ) | ||
).toHaveAttribute( 'alt', 'new value' ); | ||
} ); |
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.
test( 'should not be possible to edit the value of the protected custom fields', async ( { | ||
editor, | ||
} ) => { | ||
await editor.insertBlock( { | ||
name: 'core/paragraph', | ||
attributes: { | ||
content: 'paragraph default content', | ||
metadata: { | ||
bindings: { | ||
content: { | ||
source: 'core/post-meta', | ||
args: { key: '_protected_field' }, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
const paragraphBlock = editor.canvas.getByRole( 'document', { | ||
name: 'Block: Paragraph', | ||
} ); | ||
|
||
await expect( paragraphBlock ).toHaveAttribute( | ||
'contenteditable', | ||
'false' | ||
); | ||
} ); |
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.
test( 'should show the label of a source only registered in the server', async ( { | ||
editor, | ||
page, | ||
} ) => { | ||
await editor.insertBlock( { | ||
name: 'core/paragraph', | ||
attributes: { | ||
metadata: { | ||
bindings: { | ||
content: { | ||
source: 'core/server-source', | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ); | ||
|
||
const bindingsPanel = page.locator( | ||
'.block-editor-bindings__panel' | ||
); | ||
await expect( bindingsPanel ).toContainText( 'Server Source' ); | ||
} ); |
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 gone through the old tests and added a comment to explain where it has been moved. Aside from a couple of improvements, I believe this is almost ready. Basically, it clarifies which tests are related to custom sources and which ones to post meta, and I unify some tests because they felt redundant. Any feedback is welcome. |
Theme URI: https://github.com/wordpress/gutenberg/ | ||
Author: the WordPress team | ||
Description: Block bindings test theme. | ||
Requires at least: 5.3 |
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 requires at least: 6.7. When was custom templates introduced?
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.
Addressed in this commit: c7457d0
Description: Block bindings test theme. | ||
Requires at least: 5.3 | ||
Tested up to: 6.7 | ||
Requires PHP: 5.6 |
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.
Since WP 6.6. We only support PHP 7.2. We don't have PHP on the theme, but maybe is good to have it updated.
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.
Addressed in this commit: c7457d0
License: GNU General Public License v2 or later | ||
License URI: http://www.gnu.org/licenses/gpl-2.0.html | ||
Text Domain: block-bindings | ||
Block bindings WordPress Theme, (C) 2021 WordPress.org |
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.
2024 😆
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.
Addressed in this commit: c7457d0 😄
return newValues; | ||
}; | ||
const setValues = () => { | ||
// DO NOTHING |
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.
Maybe we should set an example of setValues, in case people use tests as examples of registering sources like you mention in the PR.
it seems it reads better the testing for the client registration APIs, which could help after making them public.
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 added an example in this commit: 6ed6f43
Makes totally sense. And is a good pattern for future sources.
Using the templates seems more optimal. There are a lot of tests, so it may save some CI time and resources. We should document at the beginning of each source test file that the templates are defining the blocks.
In my opinion, developers should use plugins to register their custom blocks, sources and custom post types. Using PHP in themes for plugin territory was an anti-pattern. You should be able to use your movies in different themes! |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
@@ -0,0 +1,55 @@ | |||
const { unlock } = | |||
wp.privateApis.__dangerousOptInToUnstableAPIsOnlyForCoreModules( |
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.
Remember to update this once they are public.
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.
68 comments, +1979 lines, -2452.
LGTM, thanks for commenting all the movements, but I also revised all the tests we already had. 😆
* WIP: Add script to block bindings tests * Add first version of block bindings test theme * Register movie CPT * Create structure for tests * Remove current specs * Share data between server and client * Add `getValues` tests * Move checks for paragraph locking * Wrap tests that should lock editing * Add setValues e2e tests * Add tests for `getFieldsList` * Add tests for rich text workflows * Add remaining use cases * Use one complete source * Add tests for CPT template * Fix label test * Add attributes panel tests * Add dropdown post meta tests * Add post meta tests for custom templates * Add e2e tests in movie post * Add template for posts * Rename specs * Fix post-meta tests * Check post meta in the frontend * Update theme definition * Populate `setValues` * Check image title locking Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org>
* WIP: Add script to block bindings tests * Add first version of block bindings test theme * Register movie CPT * Create structure for tests * Remove current specs * Share data between server and client * Add `getValues` tests * Move checks for paragraph locking * Wrap tests that should lock editing * Add setValues e2e tests * Add tests for `getFieldsList` * Add tests for rich text workflows * Add remaining use cases * Use one complete source * Add tests for CPT template * Fix label test * Add attributes panel tests * Add dropdown post meta tests * Add post meta tests for custom templates * Add e2e tests in movie post * Add template for posts * Rename specs * Fix post-meta tests * Check post meta in the frontend * Update theme definition * Populate `setValues` * Check image title locking Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org>
This reverts commit b341373.
What?
In this pull request, I'm exploring the possibility of refactoring and reorganizing the block bindings e2e tests.
Why?
The main reasons for doing that:
How?
I plan to do a few things:
privateApis
package because they aren't public yet.Testing Instructions
e2e tests should pass.