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

[Fonts API] Add tests for gutenberg_add_registered_fonts_to_theme_json() #50049

Merged

Conversation

hellofromtonya
Copy link
Contributor

@hellofromtonya hellofromtonya commented Apr 24, 2023

What?

Adds PHPUnit tests for gutenberg_add_registered_fonts_to_theme_json().

And as this new test also includes the same switch_theme() fixtures code, moved it to the TestCase for reuse and updated the other test accordingly.

Why?

  • To provide code coverage specifically for this function as it is critical for the automatic registration and enqueuing of the theme's theme.json and style variation fonts and variations.
  • To set a baseline of automated tests before improvement happens for this function.

How?

Adds a test class for this function with the following coverage:

  • Ensure an instance of WP_Theme_JSON_Gutenberg is returned.
  • Returns the given instance (bails out) when there are no fonts to add.
  • Adds registered fonts not included in the theme's theme.json.

Notes:

In writing these tests, discovered the following bugs:

The data set for #50048 is included but commented out.

Testing Instructions

CI is green.

Manual

npm run test:unit:php -- --filter Tests_Fonts_GutenbergAddRegisteredFontsToThemeJson

or all of the Fonts API tests by:

npm run test:unit:php -- --group fontsapi

@hellofromtonya hellofromtonya self-assigned this Apr 24, 2023
@hellofromtonya hellofromtonya added [Feature] Fonts API [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels Apr 24, 2023
@hellofromtonya hellofromtonya force-pushed the add/tests/gutenberg_add_registered_fonts_to_theme_json branch from 5b562dd to be957bd Compare April 25, 2023 13:28
Why? To reduce redundant tasks / code for setting up and tearing
down theme location functionality in tests that use switch_theme().
@hellofromtonya hellofromtonya force-pushed the add/tests/gutenberg_add_registered_fonts_to_theme_json branch from f76477e to c086ced Compare April 25, 2023 16:36
@hellofromtonya hellofromtonya marked this pull request as ready for review April 25, 2023 17:30
Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me.
I only have one question regarding the unused code in the data provider (please see below).
Thank you.

Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me.
I've tested this PR using the command

php vendor/bin/phpunit -c phpunit.xml.dist --filter Tests_Fonts_GutenbergAddRegisteredFontsToThemeJson

and the tests passed.
So, I approve this pull request.

@hellofromtonya
Copy link
Contributor Author

Thank you @anton-vlasenko!

@hellofromtonya hellofromtonya merged commit cbf2e36 into trunk Apr 25, 2023
@hellofromtonya hellofromtonya deleted the add/tests/gutenberg_add_registered_fonts_to_theme_json branch April 25, 2023 21:05
@github-actions github-actions bot added this to the Gutenberg 15.7 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants