Skip to content
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

Tests: fix block type notices #21928

Merged
merged 1 commit into from
Dec 18, 2024
Merged

Tests: fix block type notices #21928

merged 1 commit into from
Dec 18, 2024

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Dec 18, 2024

Context

  • Improve test setup

Summary

This PR can be summarized in the following changelog entry:

  • Improve test setup

Relevant technical choices:

When running the tests, three notices along the lines of the below are shown at the top of the test run:

PHP Notice:  Function WP_Block_Type_Registry::register was called <strong>incorrectly</strong>. Block type names must contain a namespace prefix. Example: my-plugin/my-custom-block-type Please see <a href="https://developer.wordpress.org/advanced-administration/debug/debug-wordpress/">Debugging in WordPress</a> for more information. (This message was added in version 5.0.0.) in /tmp/wordpress/wp-includes/functions.php on line 6114

The reason these notices show is that the blocks/*/*/block.json files for the Yoast custom blocks do not exist in a plain PHP based test environment in which no npm install (etc) has been run.

Now why does that cause these notices ?

  • The test bootstrap initializes the plugin with WordPress.
  • As part of the activation/initialization of the plugin, the custom blocks are being registered using the register_block_type() function.
  • The first parameter of the register_block_type() function is one of those monstrosities you find in WP - it can be various different data types:

    Block type name including namespace, or alternatively a path to the JSON file with metadata definition for the block, or a path to the folder where the block.json file is located, or a complete WP_Block_Type instance

  • The YoastSEO plugin uses the "path to the JSON file with metadata definition for the block" variant, but the files referenced in the parameter do not exist in a plain PHP test environment.
  • If WordPress can't find the file and the parameter is a string, it presumes the parameter was a "block type name including namespace", but a file path (as passed by YoastSEO) doesn't comply with the regex WP uses to validate the block type name, which cause the error notice.

To fix this, I've updated the yoast_seo_create_asset_files closure which was originally used to only create the src/generated/assets files to also handle creation of mocks for the missing block.json files.

As the PHP tests don't contain tests which are dependent on the real block.json files being in place and valid, this "hack" will side-step the notices from WP without diminishing the value of the tests.

Also note that if a dev-user already has (real) copies of these files in their development setup, those files will not be overwritten by this fix, so this shouldn't break anyone's dev environment.

Ref:

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  • Check that the above mentioned notices no longer appear when running the WP tests.

@jrfnl jrfnl added yoast cs/qa changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog labels Dec 18, 2024
@jrfnl jrfnl added this to the 24.3 milestone Dec 18, 2024
When running the tests, three notices along the lines of the below are shown at the top of the test run:
```
PHP Notice:  Function WP_Block_Type_Registry::register was called <strong>incorrectly</strong>. Block type names must contain a namespace prefix. Example: my-plugin/my-custom-block-type Please see <a href="https://developer.wordpress.org/advanced-administration/debug/debug-wordpress/">Debugging in WordPress</a> for more information. (This message was added in version 5.0.0.) in /tmp/wordpress/wp-includes/functions.php on line 6114
```

The reason these notices show is that the `blocks/*/*/block.json` files for the Yoast custom blocks do not exist in a plain PHP based test environment in which no `npm install` (etc) has been run.

Now why does that cause these notices ?

* The test bootstrap initializes the plugin with WordPress.
* As part of the activation/initialization of the plugin, the custom blocks are being registered using the `register_block_type()` function.
* The first parameter of the `register_block_type()` function is one of those monstrosities you find in WP - it can be various different data types:
    > Block type name including namespace, or alternatively a path to the JSON file with metadata definition for the block, or a path to the folder where the block.json file is located, or a complete WP_Block_Type instance
* The YoastSEO plugin uses the "path to the JSON file with metadata definition for the block" variant, but the files referenced in the parameter do not exist in a plain PHP test environment.
* If WordPress can't find the file and the parameter is a string, it presumes the parameter was a "block type name including namespace", but a file path (as passed by YoastSEO) doesn't comply with the regex WP uses to validate the block type name, which cause the error notice.

To fix this, I've updated the `yoast_seo_create_asset_files` closure which was originally used to only create the `src/generated/assets` files to also handle creation of mocks for the missing `block.json` files.

As the PHP tests don't contain tests which are dependent on the _real_ `block.json` files being in place and valid, this "hack" will side-step the notices from WP without diminishing the value of the tests.

Also note that if a dev-user already has (real) copies of these files in their development setup, those files will not be overwritten by this fix, so this shouldn't break anyone's dev environment.

Ref:
* https://developer.wordpress.org/reference/functions/register_block_type/
* https://developer.wordpress.org/reference/classes/wp_block_type_registry/register/
@jrfnl jrfnl force-pushed the JRF/tests-fix-block-type-notices branch from f527af0 to 7e4eab8 Compare December 18, 2024 15:02
@jrfnl jrfnl merged commit c365094 into trunk Dec 18, 2024
22 checks passed
@jrfnl jrfnl deleted the JRF/tests-fix-block-type-notices branch December 18, 2024 15:13
@coveralls
Copy link

Pull Request Test Coverage Report for Build 84d676640cd2fc7bd09bd2ff15e0e4f4fe880f0b

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 54.636%

Totals Coverage Status
Change from base Build 66f0345cc4407d433b29dac90f708bfcb3a4f40f: 0.0%
Covered Lines: 30125
Relevant Lines: 55490

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog yoast cs/qa
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants