Test tool and unit test improvements for 6.7

[jonsurrell]

• #60705 (6.6)
• #59647 (6.5)
• #58955 (6.4)
• #57841 (6.3)
• #56793 (6.2)
• #55652 (6.1)
• #54725 (6.0)
• #53363 (5.9)
• #52625 (5.8)
• #51802 (5.7)
• #51344 (5.6)

This ticket is for various fixes and improvements in PHPUnit tests that don't have a more specific ticket, as well as general improvements to the GitHub Actions workflows that run automated testing.

Add tests for comment handling.

These tests caught a regression during the 6.6 development cycle (with processing instruction comment tag name) that was fixed, but the tests were not merged.

Trac ticket: https://core.trac.wordpress.org/ticket/61530

Good suggestions, thanks @aaronjorbin. I've applied them.

Trac ticket: Core-61530

Add tests for comment handling.

These tests caught a regression during the 6.6 development cycle (with processing instruction comment tag name) that was fixed, but the tests were not merged.

[SergeyBiryukov]

In 58594:

Tests: Use assertSame() in WP_Interactivity_API tests.

This ensures that not only the return values match the expected results, but also that their type is the same.

Going forward, stricter type checking by using assertSame() should generally be preferred to assertEquals() where appropriate, to make the tests more reliable.

Follow-up to [57563], [57649], [57822], [57826], [57835], [58159], [58327].

See #61530.

Trac ticket: Core-61530

Add tests for comment handling.

These tests caught a regression during the 6.6 development cycle (with processing instruction comment tag name) that was fixed, but the tests were not merged.

Thanks @dmsnell, I applied those suggestions and cleaned things up. The signatures do become much simpler when we split the #comment and #funky-comment tests.

Trac ticket: Core-61530

Add tests for comment handling.

These tests caught a regression during the 6.6 development cycle (with processing instruction comment tag name) that was fixed, but the tests were not merged.

Trac ticket: Core-61530

Add tests for comment handling.

These tests caught a regression during the 6.6 development cycle (with processing instruction comment tag name) that was fixed, but the tests were not merged.

Trac ticket: Core-61530

Add tests for comment handling.

These tests caught a regression during the 6.6 development cycle (with processing instruction comment tag name) that was fixed, but the tests were not merged.

The ${ syntax for string interpolation was deprecated in PHP 8.2 and should not be used anymore.

Ref: ​https://wiki.php.net/rfc/deprecate_dollar_brace_string_interpolation

Trac ticket: https://core.trac.wordpress.org/ticket/61530
Trac ticket: https://core.trac.wordpress.org/ticket/59654

[jrf]

PR ​GH 7039 fixes a newly introduced PHP 8.2 deprecation notice.

This was not caught by running the tests as this is a deprecation notice which is thrown at compile time, not runtime.

[hellofromTonya]

Patch: ​https://github.com/WordPress/wordpress-develop/pull/7039

Before this patch: the deprecation is thrown at the start of running the PHP 8.2+ tests ([see it here](​https://github.com/WordPress/wordpress-develop/actions/runs/9957713967/job/27510405428#logs))

Not running external-http tests. To execute these, use --group external-http.

Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /var/www/tests/phpunit/tests/html-api/wpHtmlProcessorHtml5lib.php on line 257
PHPUnit 9.6.20 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.21

After this patch, the deprecation is no longer thrown ✅

Marking for and preparing the commit.

[hellofromTonya]

In 58733:

HTML API: Fix "${var} in strings" deprecation notice in html5lib test.

Changeset [58712] introduced the following compile time PHP deprecation notice on >= PHP 8.2 test runs:

Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /var/www/tests/phpunit/tests/html-api/wpHtmlProcessorHtml5lib.php on line 257
PHPUnit 9.6.20 by Sebastian Bergmann and contributors.

The ${ syntax for string interpolation was deprecated in PHP 8.2 and should not be used anymore.

Ref: ​https://wiki.php.net/rfc/deprecate_dollar_brace_string_interpolation

Follow-up to [58712].

Props jrf.
See #61530, #59654, #61576.

Committed via https://core.trac.wordpress.org/changeset/58733.

### Tests/wpInteractivityAPIFunctions: don't declare nested named function

Once the test_process_directives_when_block_is_filtered() method has run, the named test_render_block_data() function declared nested within becomes part of the global namespace, which could cause problems for other tests.

Quite apart from the fact that the name starting with test_ is confusing (as methods prefixed with test_ are supposed to be test methods to be run by PHPUnit).

Using a closure for this callback fixes the issue.

### Tests/test_wp_interactivity_data_wp_context_with_different_arrays(): use data provider

Refactor the test to use a data provider with named test cases.

This is better as:

1. One failing test will not block the other tests from running.
2. Each test is now referenced by name in any error message, making it more straight forward to see which test failed.
3. The test no longer contains multiple assertions.
4. It makes it more straight forward to add additional tests.

### Tests/test_wp_interactivity_data_wp_context_with_different_arrays_and_a_namespace(): use data provider

Refactor the test to use a data provider with named test cases.

This is better as:

1. One failing test will not block the other tests from running.
2. Each test is now referenced by name in any error message, making it more straight forward to see which test failed.
3. The test no longer contains multiple assertions.
4. It makes it more straight forward to add additional tests.

### Tests/test_wp_interactivity_data_wp_context_with_json_flags(): use data provider

Refactor the test to use a data provider with named test cases.

This is better as:

1. One failing test will not block the other tests from running.
2. Each test is now referenced by name in any error message, making it more straight forward to see which test failed.
3. The test no longer contains multiple assertions.
4. It makes it more straight forward to add additional tests.

Trac ticket: https://core.trac.wordpress.org/ticket/61530

[jrf]

PR ​GH 7040 contains some stability and maintainability improvements for the InteractivityAPI tests.

[dmsnell]

In 58734:

HTML API: Add tests confirming comment behavior in HTML Processor.

There was a bug-fix late in the 6.6 cycle in the HTML Processor which
resolved an issue with the wrong name being reported when paused at
processing-instruction look-alike invalid comments, but no tests were
added to enforce the correct behaviors.

This patch adds the missing tests.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/6765
Discussed in https://core.trac.wordpress.org/ticket/61530

Follow-up to [58304], [58558].

Props dmsnell, jonsurrell.
See #61530.

Merged in [58734]
​https://github.com/wordpress/wordpress-develop/commit/bd61b0bf236ed9c04bbbb50fd9b34d1b94970c3c

Trac ticket: Core-61530

Add tests for comment handling.

These tests caught a regression during the 6.6 development cycle (with processing instruction comment tag name) that was fixed, but the tests were not merged.

[hellofromTonya]

Replying to jrf:

PR ​GH 7040 contains some stability and maintainability improvements for the InteractivityAPI tests.

Reviewed. Approved. In the process of prepping the commit.

[hellofromTonya]

In 58738:

Tests: Don't declare nested named function in Tests_Interactivity_API_wpInteractivityAPIFunctions.

Once the test_process_directives_when_block_is_filtered() method has run, the named test_render_block_data() function declared nested within becomes part of the global namespace, which could cause problems for other tests.

Quite apart from the fact that the name starting with test_ is confusing (as methods prefixed with test_ are supposed to be test methods to be run by PHPUnit).

Using a closure for this callback fixes the issue. Declared as static for a micro-optimization.

Follow-up to [57826].

Props jrf, hellofromTonya.
See #61530.

[hellofromTonya]

In 58739:

Tests: Use data provider in Tests_Interactivity_API_wpInteractivityAPIFunctions.

Refactors the following tests to use a data provider with named test cases:

• test_wp_interactivity_data_wp_context_with_different_arrays()
• test_wp_interactivity_data_wp_context_with_different_arrays_and_a_namespace()
• test_wp_interactivity_data_wp_context_with_json_flags()

This is better as:

1. One failing test will not block the other tests from running.
2. Each test is now referenced by name in any error message, making it more straight forward to see which test failed.
3. The test no longer contains multiple assertions.
4. It makes it more straight forward to add additional tests.

Follow-up to [58594], [58234], [57762], [57743], [57742], [57563].

Props jrf.
See #61530.

Committed via https://core.trac.wordpress.org/changeset/58738 and https://core.trac.wordpress.org/changeset/58739.

Thanks @hellofromtonya!

[SergeyBiryukov]

In 58803:

Tests: Use more specific assertions in get_comment_author() tests.

Follow-up to [58335].

See #61530.

[hellofromTonya]

In 58919:

Tests: Remove WP_Term::$filter property unset() within term tests.

Removes the unset() of the WP_Term::$filter property within the term tests.

Why?

Prior to the introduction of WP_Term, the term was added to the cache when its filter property was empty. To test the cache, the tests unset this property to trigger wp_cache_add() in get_term(). [34997] changed that behavior to trigger wp_cache_add() when the term was not found after wp_cache_get() (i.e. happened in WP_Term::get_instance()).

Unsetting the filter property is and was not needed. Prior to WP_Term, the condition was an empty value. With WP_Term, the filter property is no longer part of the conditional logic for caching.

Follow-up to [34997], [30954], [34035].

See #61890, #61530.

[hellofromTonya]

In 58920:

Tests: Remove 'errors' assertion when not WP_Error in Tests_Term_WpInsertTerm.

Removes the assertion for 'errors' being empty when the instance is WP_Term and not WP_Error. This property exists on WP_Error.

This assertion always passed because it was checking a dynamic property on WP_Term that does not exist and is not added within Core. Thus, this assertion is not needed and fails with dynamic property deprecations.

Follow-up to [51403], [34646], [29830].

See #61890, #61530.

[SergeyBiryukov]

In 58954:

Tests: Clarify description for unregister_setting() test with an unknown setting.

Follow-up to [56817].

See #61530.

[SergeyBiryukov]

In 58968:

Tests: Add a unit test for get_metadata() with a non-existing object ID.

Follow-up to [48658], [50641], [58962].

Props rodrigosprimo, jrf.
See #61530, #61608.

[peterwilsoncc]

In 59015:

Taxonomy: Test inserting a child term flushes queries by term ID.

Adds a test to ensure that interting a child term invalidates the cache of a get_terms() query by the parent ID.

Props Dekadinious, peterwilsoncc.
See #62031, #61530.

Composer 1.10.0 introduced a lock config option, which, when set to false will prevent a composer.lock file from being created and will ignore it when one exists.

This is a useful option for packages like WordPress where the lock file has no meaning.

It also makes life more straight-forward for contributors as they don't have to remember that for this repo they should use composer update instead of composer install. Both will now work the same.

Refs:
​https://getcomposer.org/doc/06-config.md#lock

Trac ticket: https://core.trac.wordpress.org/ticket/61530

[jrf]

I've opened ​GH PR #7362 with a quality of life improvement for contributors using Composer to locally run the tests.

Tests which can not run because a prerequisite is not fulfilled should be marked as "skipped", not as _failed_ (as they haven't failed, they didn't run).

Now, I can imagine the choice to mark the tests as "failed" may have been to prevent contributors missing the message about the need to install the Importer plugin. If that's the case, I'd suggest adding a file_exists() check for the importer plugin to the test bootstrap.php file and exiting the test run if that prerequisite is not fulfilled, along the same lines as the check that the database connection can be made and that the correct version of the Polyfills is installed.

I'll happily update the test bootstrap to add this, but would like some more opinions on whether or not this is really a necessity.

Trac ticket: https://core.trac.wordpress.org/ticket/61530

[jrf]

I've opened ​GH PR #7363 to improve the output from PHPUnit by not marking tests which should be skipped due to an unfulfilled test requirement as failed.

### WP_UnitTestCase_Base: introduce a new assertion and a new helper method

Introduces WP_UnitTestCase_Base::assertSamePathIgnoringDirectorySeparators() and an associated helper method WP_UnitTestCase_Base::normalizeDirectorySeparatorsInPath() to allow for comparing two file path strings independently of OS-specific differences.

The normalization is done in a separate method to also allow this method to be used for path normalization within test methods themselves, like for normalizing a group of paths in an array.

The pretty specific method name for the helper (normalizeDirectorySeparatorsInPath()) is an attempt to prevent naming conflicts with methods which may exist in plugin test suites build on top of the WP Core test suite.

### Tests: start using the new assertion and helper method

Trac ticket: https://core.trac.wordpress.org/ticket/61530

[jrf]

I've opened ​GH PR #7364 to stabilize some tests which would pass on *nix systems, but would structurally fail on Windows systems.

This is on the one hand a quality of life improvement for contributors on Windows running the PHPUnit tests locally. On the other hand, it paves the way towards being able to run the tests on Windows in CI at some point in the future.

Trac ticket: https://core.trac.wordpress.org/ticket/61530

[jrf]

I've opened ​GH PR #7365 with another two fixes to resolve test failures on Windows.

[hellofromTonya]

In 59054:

Tests: Fix Tests_Theme tests to run (and pass) cross-OS.

Uses DIRECTORY_SEPARATOR in closures for cross-OS differences.

Follow-up to [56635].

Props jrf.
See #61530.

Committed via https://core.trac.wordpress.org/changeset/59054 ✅

[hellofromTonya]

Hmm, this changeset is not showing in Trac, but ​is in the build. Manually copying the details here for reference.

In 59057:

Tests: Introduce assertion for comparing file paths independent of OS-specifics.

Introduces WP_UnitTestCase_Base::assertSamePathIgnoringDirectorySeparators() and an associated helper method WP_UnitTestCase_Base::normalizeDirectorySeparatorsInPath() to allow for comparing two file path strings independently of OS-specific differences.

The normalization is done in a separate method to also allow this method to be used for path normalization within test methods themselves, like for normalizing a group of paths in an array.

The pretty specific method name for the helper (normalizeDirectorySeparatorsInPath()) is an attempt to prevent naming conflicts with methods which may exist in plugin test suites build on top of the WP Core test suite.

Props jrf, hellofromTonya.
See #61530.

[hellofromTonya]

In 59061:

Tests: Use file paths independent of OS-specifics assertion or helper.

Use WP_UnitTestCase_Base::assertSamePathIgnoringDirectorySeparators() and WP_UnitTestCase_Base::normalizeDirectorySeparatorsInPath() in existing tests.

Follow-up to [59057], [57753], [57215], [56635], [48937], [25002].

Props jrf.
See #61530.

• ​https://github.com/WordPress/wordpress-develop/pull/7364/commits/0e965d96ff42447ebd4c1858c777df7bcd933fdd committed via https://core.trac.wordpress.org/changeset/59057 ✅
• ​https://github.com/WordPress/wordpress-develop/pull/7364/commits/309ce819670e78072b074fa5cb8a8db22dd40583 committed via https://core.trac.wordpress.org/changeset/59061 ✅

[SergeyBiryukov]

In 59082:

Build/Test Tools: Prevent Composer lock file from being created.

Composer 1.10.0 introduced a lock config option, which, when set to false will prevent a composer.lock file from being created and will ignore it when one exists.

This is a useful option for packages like WordPress where the lock file has no meaning.

It also makes life more straightforward for contributors as they don't have to remember that for this repo they should use composer update instead of composer install. Both will now work the same.

Reference: ​Composer Documentation: Config: lock.

Follow-up to [51543].

Props jrf.
See #61530.

Thanks for the PR! Merged in r59082.

Thanks @SergeyBiryukov!

[SergeyBiryukov]

In 59085:

Build/Test Tools: Check if the WordPress Importer plugin is installed in test bootstrap.

If a hard requirement for the test suite is not fulfilled, running the tests should be blocked from the test bootstrap. A test should only fail when it doesn't produce the expected result.

Since the WordPress Importer plugin is considered a hard requirement for the test suite at this time, this commit moves the check whether the plugin is installed from individual tests to the test bootstrap.

Includes defining a global constant for the path to the file for reuse in the tests.

Reference: ​Core Contributor Handbook: The Code Repository (Git): Unit Tests.

Follow-up to [40531], [40532], [41090], [41169], [48592], [49535], [49571].

Props jrf, hellofromTonya.
See #61530.

Thanks for the PR! Merged in r59085.

[bjorsch]

FYI, the latest PR merged has broken plugin integration testing following the instructions at ​https://make.wordpress.org/cli/handbook/misc/plugin-unit-tests/. See #62106 for details.

[jrf]

@bjorsch I just checked and my first thought was adding a check for WP_RUN_CORE_TESTS, but kudos to @SergeyBiryukov, that check is already in place in the patch as committed.

So the short of it, as far as I can see, is that plugin tests should not be broken by this commit. Plugin integration tests should not set the WP_RUN_CORE_TESTS PHP constant, or if they do, it should be set to 0.

Typically, the constant is set like below from the phpunit.xml[.dist] file when running the Core tests.

	<php>
		<const name="WP_RUN_CORE_TESTS" value="1" />
	</php>

I've also done a test run with a plugin using integration tests with WP Core and the tests run just fine for the plugin I tested with.

I've also checked the template file used by the WP-CLI scaffold command for the ​phpunit.xml.dist file and it also doesn't include the constant.

All in all, this sounds like a problem with the plugin integration test set up of that particular plugin, not with WP Core.
@bjorsch Could you please check for the tests where you noticed the failure that the constant is not being set ?

Thanks @SergeyBiryukov! And thanks for adding the WP_RUN_CORE_TESTS check to the condition too.

[ramonopoly]

Are there any updates required in the Gutenberg repo?

The PHP unit tests are failing with the following error, which corresponds to that committed in https://core.trac.wordpress.org/changeset/59085

[25-Sep-2024 04:06:39 UTC] PHP Warning:  Constant WP_DEBUG already defined in /wordpress-phpunit/wp-tests-config.php on line 119
The test suite requires the WordPress Importer plugin to be available in the `/data/plugins/` directory. See: https://make.wordpress.org/core/handbook/contribute/git/#unit-tests

[jrf]

@ramonopoly IF Gutenberg runs with WP_RUN_CORE_TESTS=1, the first thing to do is figure out if that is actually the right setup. If yes: in that case, the importer plugin should be installed in the WP tests/phpunit/data/plugins directory. If not: remove the constant or set it to 0.

This is basically the same as my previous answer. The Core tests are set up to allow for plugin integration tests, but it is not Core's responsibility if those integration tests are not wired correctly.

[jrf]

Note: if something else is going on and this is not related to the constant, please provide more information so we can figure out whether this needs to be reverted or changed. Based on the current info + patch, I see no reason for that, but new information may change that.

[ramonopoly]

Thanks for the quick reply and context @jrf

Gutenberg does have WP_RUN_CORE_TESTS set to true in the phpunit bootstrap.php. If, after testing, any more info arises that's pertinent to this ticket I'll update, but it sounds like the fix is plugin-specific.

Thank you!

[bjorsch]

Replying to jrf:

@bjorsch I just checked and my first thought was adding a check for WP_RUN_CORE_TESTS, but kudos to @SergeyBiryukov, that check is already in place in the patch as committed.

The check wasn't included in r59085, which was the latest when I wrote that comment. Sergey added it in r59086 to fix the problem I reported. 🙂

[jrf]

@bjorsch Ah, yes, I only noticed that was a second commit after I replied. I presume I can conclude that solved it for you though ?

[bjorsch]

Replying to jrf:

I presume I can conclude that solved it for you though ?

Yes, it did. 🙂

[peterwilsoncc]

In 59116:

Build/Test Tools: Re-order assertion parameters query block tests.

Corrects the order of the expected and actual values in several tests of the build_query_vars_from_query_block() function.

See #61530.

Additional tests for default themes:

• Ensure the zip test matrix includes all themes
• Ensure test suite and WP_Theme default lists match
• Ensure the constant matches the list in WP_Theme

Trac ticket: Core-61530

[SergeyBiryukov]

In 59163:

Tests: Bring some consistency to links_add_base_url() and links_add_target() tests.

Includes:

• Correcting the test class name as per the naming conventions.
• Documenting data provider values using hash notation.
• Passing the $attrs parameter to the function if not null.

Follow-up to [26328], [55563], [59162].

See #61530.

[peterwilsoncc]

In 59186:

Tests/Build Tools: Improve tests for bundled themes.

Introduce two new tests relating to bundled themes:

1. Ensure the list of tested themes matches the list of themes defined in WP_Theme
2. Ensure the run time value of WP_DEFAULT_THEME is included in the list of themes defined in WP_Theme

See #61530.

r59186 / ​https://github.com/WordPress/wordpress-develop/commit/023d3c4b3dfdcb59bb235d79697a6d55698719b8

Trac ticket: Core-61530

[desrosj]

In 59212:

Build/Test Tools: Don’t install Gutenberg for E2E tests in 6.4.

The minimum required version of WordPress for the Gutenberg plugin was recently raised to 6.5.

This updates the 6.4 branch to skip installing and activating Gutenberg during E2E testing as has already been done for older branches.

See #61530.