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

Nav link block PHP tests replace prefixed function with non-prefixed #40657

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Apr 27, 2022

What?

Replaces the usage of prefixed gutenberg_render_block_core_navigation_link with non-prefixed render_block_core_navigation_link in the PHP Unit tests.

Here's an example where they are failing:

https://github.com/WordPress/gutenberg/runs/6195031409?check_suite_focus=true

Why?

It looks like the Nav Link block PHP Unit tests have been relying on the gutenberg_ prefixed version of the block's render function.

gutenberg_render_block_core_navigation_link(

However, this function does not exist in Gutenberg Core so I assume it existed in WP Core. It looks like it was recently removed and thus causes a bug in the PHP unit tests.

How?

Just renaming.

Testing Instructions

Run PHP unit tests for phpunit/class-block-library-navigation-link-test.php. Check no failures.

Screenshots or screencast

Example of failing tests
Screen Shot 2022-04-27 at 16 39 42

@getdave getdave self-assigned this Apr 27, 2022
@getdave getdave added the [Block] Navigation Link Affects the Navigation Link Block label Apr 27, 2022
@getdave getdave marked this pull request as ready for review April 29, 2022 09:42
@adamziel
Copy link
Contributor

  • In the plugin, this function has been called render_block_core_navigation_link for a while.
  • I noticed it's renamed to gutenberg_render_block_core_navigation_link during the plugin build process.
  • In my local test environment, both render_block_core_navigation_link and gutenberg_render_block_core_navigation_link exists and I'm unable to reproduce the error.

Super weird! cc @youknowriad @gziolo

@adamziel adamziel merged commit 0b19b54 into trunk Apr 29, 2022
@adamziel adamziel deleted the fix/nav-link-block-php-unit-function-ref branch April 29, 2022 11:15
@github-actions github-actions bot added this to the Gutenberg 13.2 milestone Apr 29, 2022
@gziolo
Copy link
Member

gziolo commented Apr 29, 2022

In my local test environment, both render_block_core_navigation_link and gutenberg_render_block_core_navigation_link exists and I'm unable to reproduce the error.

It's only the case when you run npm run build because webpack will copy the file to build and rename the function 😄

That's the risk of doing too much magic in the build process.

adamziel pushed a commit that referenced this pull request May 3, 2022
…xed render_block_core_navigation_link in the unit tests (#40657)
@adamziel
Copy link
Contributor

adamziel commented May 3, 2022

I cherry picked this change into wp/6.0 branch to be included in WordPress 6.0 RC1: 12a4722

@georgeh georgeh added [Type] Bug An existing feature does not function as intended [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Link Affects the Navigation Link Block [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants