Make fonts test files use Core approach #53856

hellofromtonya · 2023-08-21T20:19:54Z

What?

Makes the fonts tests ready for Core merge by eliminating the need to rename the test files.

Why?

To make it easier to merge these tests into Core.

WordPress Core's PHPUnit tests:

are contained within tests/phpunit/tests/ directory.
do not add -test.php to the test filename.
it uses <directory suffix=".php"> to identify where the tests are located.

By moving the files into phpunit/tests, Gutenberg's tests will no longer need to add the -test.php suffix to the test filenames. This means the test files will not have to be renamed when merging them into WordPress Core.

How?

Relocates the fonts tests into phpunit/tests/
Removes -test from each test filename.
Removes testcase filename to base.php to match Core.
Adds the directive to xml files.
Adds 2 constants to identify where the location of data and fixtures directories.

Testing Instructions

There should be no change in the number of tests that run before and after this PR.

To run all of the tests, run the following:

npm run test:unit:php

should produce the same results before and after this PR. (Try it on trunk and this PR.)

For the fonts tests only, run the following:

npm run test:unit:php:base -- --group fonts

which should produce the exact same results before and after this PR. (Try it on trunk and this PR.)

* Moves the files into phpunit/tests/ directory to match Core. * Removes the `-test.php` suffix from each test file. * Adds the directive to find the test files in phpunit/tests/ (to match how Core does it). * Adds GUTENBERG_DIR_TESTDATA and GUTENBERG_DIR_TESTFIXTURES constants to make it easier on tests to point to these locations.

github-actions · 2023-08-21T20:21:34Z

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files

❔ phpunit/bootstrap.php
❔ phpunit/multisite.xml
❔ phpunit/tests/fonts-api/base.php
❔ phpunit/tests/fonts-api/bc-layer/base.php
❔ phpunit/tests/fonts-api/bc-layer/bc-layer-tests-dataset.php
❔ phpunit/tests/fonts-api/bc-layer/gutenbergFontsApiBcLayer/isDeprecatedStructure.php
❔ phpunit/tests/fonts-api/bc-layer/gutenbergFontsApiBcLayer/migrateDeprecatedStructure.php
❔ phpunit/tests/fonts-api/bc-layer/wpRegisterWebfonts.php
❔ phpunit/tests/fonts-api/bc-layer/wpWebfonts/getAllWebfonts.php
❔ phpunit/tests/fonts-api/bc-layer/wpWebfonts/getFontSlug.php
❔ phpunit/tests/fonts-api/bc-layer/wpWebfonts/getRegisteredWebfonts.php
❔ phpunit/tests/fonts-api/bc-layer/wpWebfonts/registerWebfont.php
❔ phpunit/tests/fonts-api/wp-fonts-tests-dataset.php
❔ phpunit/tests/fonts-api/wpDeregisterFontFamily.php
❔ phpunit/tests/fonts-api/wpDeregisterFontVariation.php
❔ phpunit/tests/fonts-api/wpEnqueueFontVariations.php
❔ phpunit/tests/fonts-api/wpEnqueueFonts.php
❔ phpunit/tests/fonts-api/wpFonts.php
❔ phpunit/tests/fonts-api/wpFonts/add.php
❔ phpunit/tests/fonts-api/wpFonts/addFontFamily.php
❔ phpunit/tests/fonts-api/wpFonts/addVariation.php
❔ phpunit/tests/fonts-api/wpFonts/dequeue.php
❔ phpunit/tests/fonts-api/wpFonts/doItem.php
❔ phpunit/tests/fonts-api/wpFonts/doItems.php
❔ phpunit/tests/fonts-api/wpFonts/enqueue.php
❔ phpunit/tests/fonts-api/wpFonts/getEnqueued.php
❔ phpunit/tests/fonts-api/wpFonts/getProviders.php
❔ phpunit/tests/fonts-api/wpFonts/getRegistered.php
❔ phpunit/tests/fonts-api/wpFonts/query.php
❔ phpunit/tests/fonts-api/wpFonts/registerProvider.php
❔ phpunit/tests/fonts-api/wpFonts/remove.php
❔ phpunit/tests/fonts-api/wpFonts/removeFontFamily.php
❔ phpunit/tests/fonts-api/wpFonts/removeVariation.php
❔ phpunit/tests/fonts-api/wpFontsProviderLocal.php
❔ phpunit/tests/fonts-api/wpFontsResolver/addMissingFontsToThemeJson.php
❔ phpunit/tests/fonts-api/wpFontsResolver/enqueueUserSelectedFonts.php
❔ phpunit/tests/fonts-api/wpFontsResolver/registerFontsFromThemeJson.php
❔ phpunit/tests/fonts-api/wpFontsUtils/convertFontFamilyIntoHandle.php
❔ phpunit/tests/fonts-api/wpFontsUtils/convertVariationIntoHandle.php
❔ phpunit/tests/fonts-api/wpFontsUtils/getFontFamilyFromVariation.php
❔ phpunit/tests/fonts-api/wpFontsUtils/isDefined.php
❔ phpunit/tests/fonts-api/wpPrintFonts.php
❔ phpunit/tests/fonts-api/wpRegisterFontProvider.php
❔ phpunit/tests/fonts-api/wpRegisterFonts.php
❔ phpunit/tests/fonts/font-face/base.php
❔ phpunit/tests/fonts/font-face/wp-font-face-tests-dataset.php
❔ phpunit/tests/fonts/font-face/wpFontFace/generateAndPrint.php
❔ phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php
❔ phpunit/tests/fonts/font-face/wpPrintFontFaces.php
❔ phpunit/tests/fonts/font-library/class-wp-rest-font-library-controller.php
❔ phpunit/tests/fonts/font-library/wpFontFamily/__construct.php
❔ phpunit/tests/fonts/font-library/wpFontFamily/base.php
❔ phpunit/tests/fonts/font-library/wpFontFamily/getData.php
❔ phpunit/tests/fonts/font-library/wpFontFamily/getDataAsJson.php
❔ phpunit/tests/fonts/font-library/wpFontFamily/getFontPost.php
❔ phpunit/tests/fonts/font-library/wpFontFamily/hasFontFaces.php
❔ phpunit/tests/fonts/font-library/wpFontFamily/install.php
❔ phpunit/tests/fonts/font-library/wpFontFamily/uninstall.php
❔ phpunit/tests/fonts/font-library/wpFontFamilyUtils/getFilenameFromFontFace.php
❔ phpunit/tests/fonts/font-library/wpFontFamilyUtils/hasFontMimeType.php
❔ phpunit/tests/fonts/font-library/wpFontFamilyUtils/mergeFontsData.php
❔ phpunit/tests/fonts/font-library/wpFontLibrary/getFontsDir.php
❔ phpunit/tests/fonts/font-library/wpFontLibrary/setUploadDir.php

hellofromtonya · 2023-08-21T20:36:17Z

phpunit.xml.dist

@@ -14,6 +14,7 @@
 	<testsuites>
 		<testsuite name="default">
 			<directory suffix="-test.php">./phpunit/</directory>
+			<directory suffix=".php">./phpunit/tests/</directory>


WordPress Core does it this way https://github.com/WordPress/wordpress-develop/blob/trunk/phpunit.xml.dist#L17.

I left the original <directory> directive on purpose to:

allow all other tests to run, until they are also moved into this Core specific tests directory.

avoid disruption in existing PRs.

hellofromtonya · 2023-08-21T20:38:24Z

phpunit/bootstrap.php

+define( 'GUTENBERG_DIR_TESTDATA', __DIR__ . '/data/' );
+define( 'GUTENBERG_DIR_TESTFIXTURES', __DIR__ . '/fixtures/' );


Core uses DIR_TESTDATA to point to its tests' data directory. This is a constant that will need to be changed when merging tests into Core.

Could explore overwriting DIR_TESTDATA instead. But there may be some tests that truly need to use Core's data directory and contents.

This is a constant that will need to be changed when merging tests into Core.

I'm wondering if the necessity of renaming the constant should be documented somewhere.
I can't seem to find an appropriate page in the Gutenberg developer documentation where the requirement to rename the constant could be added.
Probably, considering adding a new phpunit/README.md file could be an option, similar to the README.md file in the lib/ folder.
What's your opinion on this matter, @hellofromtonya?

Good thinking @anton-vlasenko. I'm thinking about a separate follow-up PR that updates or adds a doc page for the Gutenberg handbook.

Why separate PR?
This PR is focused on only the fonts components, rather than all of the tests. A separate scope of work can be targeted at providing testing instructions and guidance for tests that are intended to be merged into Core. I suspect there's none currently available. Thus, the handbook/docs scope of work might be more involved.

For example, we'll want to document the new @core-merge annotation with Core merge instructions.

A separate scope of work can be targeted at providing testing instructions and guidance for tests that are intended to be merged into Core. I suspect there's none currently available. Thus, the handbook/docs scope of work might be more involved.

That's a good point, @hellofromtonya. I agree.
Doing it within the scope of another PR is fine with me.

Issue for documenting the new @core-merge annotation: #54041

hellofromtonya · 2023-08-21T20:54:23Z

Hey @matiasbenedetto I added you to the reviewers list to ensure you are aware of this change. Please don't feel like you have to do a review, as I know you are focused on the Font Library.

anton-vlasenko · 2023-08-22T15:36:30Z

Test Report

Environment

WordPress: 6.4-alpha-56267-src
PHP: 8.0.29
Server: Apache/2.4.57 (Unix) PHP/8.0.29
Database: MySQLi (Server: 5.7.43 / Client: mysqlnd 8.0.29)
Browser: Safari 16.6 (macOS)
Theme: Twenty Twenty-Three 1.1
MU-Plugins: None activated
Plugins:
- Gutenberg 16.5.0-rc.1

Actual Results

✅ 1. When running php vendor/bin/phpunit --group fonts, all the tests passed. The quantity of tests hasn't changed compared to running the tests on the trunk branch.
✅ 2. When running php vendor/bin/phpunit, not all tests passed. However, this seems to be related to my local environment, as the PR passed GitHub CI checks. The quantity of failed tests hasn't changed compared to trunk code:
Tests: 892, Assertions: 1354, Errors: 1, Failures: 5, Warnings: 5, Skipped: 1 in both cases. So, this is ✅.

anton-vlasenko

This PR LGTM (test report).
However, I have a question (please see below):

anton-vlasenko · 2023-08-22T15:11:35Z

phpunit/bootstrap.php

+define( 'GUTENBERG_DIR_TESTDATA', __DIR__ . '/data/' );
+define( 'GUTENBERG_DIR_TESTFIXTURES', __DIR__ . '/fixtures/' );


This is a constant that will need to be changed when merging tests into Core.

I'm wondering if the necessity of renaming the constant should be documented somewhere.
I can't seem to find an appropriate page in the Gutenberg developer documentation where the requirement to rename the constant could be added.
Probably, considering adding a new phpunit/README.md file could be an option, similar to the README.md file in the lib/ folder.
What's your opinion on this matter, @hellofromtonya?

anton-vlasenko

LGTM.

gziolo · 2023-08-31T06:03:44Z

I like how we are getting closer to how WordPress core is structured to make the backports easier.

hellofromtonya · 2023-08-31T18:55:50Z

Thanks! Me too :)

One more step towards seamless Core merges.

Rename testcase to base.php

fafda2f

hellofromtonya added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Aug 21, 2023

hellofromtonya commented Aug 21, 2023

View reviewed changes

hellofromtonya marked this pull request as ready for review August 21, 2023 20:53

hellofromtonya requested review from anton-vlasenko and matiasbenedetto August 21, 2023 20:53

hellofromtonya mentioned this pull request Aug 21, 2023

Server-Side Font Face CSS Generation and Printing: Ongoing Roadmap (formerly the Fonts API) #41479

Closed

5 tasks

anton-vlasenko reviewed Aug 22, 2023

View reviewed changes

anton-vlasenko approved these changes Aug 22, 2023

View reviewed changes

hellofromtonya merged commit f0ff5f5 into trunk Aug 22, 2023

hellofromtonya deleted the try/phpunit-tests-location-core-parity branch August 22, 2023 15:57

github-actions bot added this to the Gutenberg 16.6 milestone Aug 22, 2023

anton-vlasenko mentioned this pull request Aug 29, 2023

Document the GUTENBERG_DIR_TESTDATA constant and the @core-merge tag #54041

Open

vcanales mentioned this pull request Aug 30, 2023

Changelog tool: update Automated Testing label #54070

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make fonts test files use Core approach #53856

Make fonts test files use Core approach #53856

hellofromtonya commented Aug 21, 2023 •

edited

Loading

github-actions bot commented Aug 21, 2023 •

edited

Loading

hellofromtonya Aug 21, 2023

hellofromtonya Aug 21, 2023

anton-vlasenko Aug 22, 2023

hellofromtonya Aug 22, 2023

hellofromtonya Aug 22, 2023

anton-vlasenko Aug 22, 2023

anton-vlasenko Aug 30, 2023

hellofromtonya commented Aug 21, 2023

anton-vlasenko commented Aug 22, 2023

anton-vlasenko left a comment

anton-vlasenko Aug 22, 2023

anton-vlasenko left a comment

gziolo commented Aug 31, 2023

hellofromtonya commented Aug 31, 2023

		define( 'GUTENBERG_DIR_TESTDATA', __DIR__ . '/data/' );
		define( 'GUTENBERG_DIR_TESTFIXTURES', __DIR__ . '/fixtures/' );

Make fonts test files use Core approach #53856

Make fonts test files use Core approach #53856

Conversation

hellofromtonya commented Aug 21, 2023 • edited Loading

What?

Why?

How?

Testing Instructions

github-actions bot commented Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hellofromtonya commented Aug 21, 2023

anton-vlasenko commented Aug 22, 2023

Test Report

Environment

Actual Results

anton-vlasenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anton-vlasenko left a comment

Choose a reason for hiding this comment

gziolo commented Aug 31, 2023

hellofromtonya commented Aug 31, 2023

hellofromtonya commented Aug 21, 2023 •

edited

Loading

github-actions bot commented Aug 21, 2023 •

edited

Loading