Block Bindings: Refactor e2e tests #65526
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SantosGuillamot
added
[Type] Automated Testing
|
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
|
SantosGuillamot
force-pushed
the
update/reorganize-block-bindings-e2e-tests
branch
from
104ab94
to
c2ce926
Compare
SantosGuillamot
force-pushed
the
update/reorganize-block-bindings-e2e-tests
branch
from
7fa7dd2
to
74b3200
Compare
SantosGuillamot
commented
Sep 27, 2024
Comment on lines
-44
to
-67
| 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.
Comment on lines
-69
to
-90
| 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.
Comment on lines
-92
to
-136
| 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.
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
Comment on lines
-138
to
-183
| 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.
Comment on lines
-186
to
-207
| 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.
Comment on lines
-2233
to
-2286
| 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.
Comment on lines
-2288
to
-2346
| 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.
Comment on lines
-2348
to
-2392
| 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.
Comment on lines
-2394
to
-2419
| 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.
Comment on lines
-2428
to
-2449
| 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.
|
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.
I think it requires at least: 6.7. When was custom templates introduced?
There was a problem hiding this comment.
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.
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.
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.
Addressed in this commit: c7457d0 😄
| return newValues; | ||
| }; | ||
| const setValues = () => { | ||
| // DO NOTHING |
There was a problem hiding this comment.
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.
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! |
SantosGuillamot
requested review from
ntwb,
nerrad and
ajitbohra
as code owners
|
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.
Remember to update this once they are public.
cbravobernal
approved these changes
Sep 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
privateApispackage because they aren't public yet.Testing Instructions
e2e tests should pass.