Test tool and unit test improvements for 5.9

[desrosj]

Previously:

• #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.

[SergeyBiryukov]

In 51333:

Tests: Move loading compatibility layers for PHPUnit 6+ and 7.5+ closer together.

Follow-up to [40536], [44701], [50986].

See #53363.

[SergeyBiryukov]

In 51335:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( is_array( ... ) ) with assertIsArray() to use native PHPUnit functionality.

Follow-up to [51331].

See #53363.

[SergeyBiryukov]

In 51337:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( is_object( ... ) ) with assertIsObject() to use native PHPUnit functionality.

Follow-up to [51331], [51335].

See #53363.

[SergeyBiryukov]

In 51367:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertSame( [number], count( ... ) ) with assertCount() to use native PHPUnit functionality.

Follow-up to [51335], [51337].

See #53363.

[SergeyBiryukov]

In 51368:

Coding Standards: Fix WPCS issues in [51367].

This fixes an "Expected 1 space after comma in argument list; 2 found" error.

See #53363.

[SergeyBiryukov]

In 51397:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( isset( ... ) ) with assertArrayHasKey() to use native PHPUnit functionality.

Follow-up to [51335], [51337], [51367].

See #53363.

[SergeyBiryukov]

In 51403:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( empty( ... ) ) with assertEmpty() to use native PHPUnit functionality.

Follow-up to [51335], [51337], [51367], [51397].

See #53363.

[SergeyBiryukov]

In 51404:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( in_array( ... ) ) with assertContains() to use native PHPUnit functionality.

Follow-up to [51335], [51337], [51367], [51397], [51403].

Props hellofromTonya, jrf, SergeyBiryukov.
Fixes #53123. See #53363.

[SergeyBiryukov]

In 51405:

Coding Standards: Fix WPCS issue in [51404].

This fixes an "Expected 1 spaces before closing parenthesis; 0 found" error.

See #53363.

[SergeyBiryukov]

In 51436:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( is_a( ... ) ) with assertInstanceOf() to use native PHPUnit functionality.

Follow-up to [51335], [51337], [51367], [51397], [51403], [51404].

See #53363.

[SergeyBiryukov]

In 51438:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( is_numeric( ... ) ) with assertIsNumeric() to use native PHPUnit functionality.

Follow-up to [51335], [51337], [51367], [51397], [51403], [51404], [51436].

See #53363.

[SergeyBiryukov]

In 51448:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( is_string( ... ) ) with assertIsString() to use native PHPUnit functionality.

Follow-up to [51335], [51337], [51367], [51397], [51403], [51404], [51436], [51438].

See #53363.

[SergeyBiryukov]

In 51449:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertSame( 0, strpos( ... ) ) with assertStringStartsWith() to use native PHPUnit functionality.

Follow-up to [51335], [51337], [51367], [51397], [51403], [51404], [51436], [51438], [51448].

See #53363.

[SergeyBiryukov]

In 51450:

Tests: Correct the test for autosaving a post with Ajax.

wp_autosave() only updates drafts and auto-drafts created by the current user if the post is not locked.

As a result of previous Ajax test refactoring, setting the current user and creating a test post ended up in different methods, with the user being set after the post is already created.

This resulted in the test post being created with the post_author field set to zero, and the current user check in wp_autosave() failed. Instead of updating the original post as the test intended, it created a new autosave.

The test only passed accidentally due to assertGreaterThanOrEqual() not performing a strict type check.

Follow-up to [26995], [35311].

See #53363.

[SergeyBiryukov]

In 51451:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( strpos( ... ) > 0 ) with assertStringContainsString() to use native PHPUnit functionality.

Going forward, these methods introduced in PHPUnit 7.5 should be used for similar assertions:

• assertStringContainsString()
• assertStringNotContainsString()

As WordPress currently uses PHPUnit 5.7.x to run tests on PHP 5.6, polyfills for these methods are now added to the WP_UnitTestCase class for PHPUnit < 7.5.

Follow-up to [51335], [51337], [51367], [51397], [51403], [51404], [51436], [51438], [51448], [51449].

See #53363.

[SergeyBiryukov]

In 51452:

Tests: Require the WP_REST_Test_Controller class in WP_REST_Controller tests.

This avoids a "Class not found" PHP fatal error when running these tests separately.

Follow-up to [38832].

See #53363.

[SergeyBiryukov]

In 51453:

Tests: Use more appropriate assertions in rest_sanitize_request_arg() tests.

This replaces instances of assertSame( true, ... ) with assertTrue() to use native PHPUnit functionality.

Follow-up to [38832].

See #53363.

[SergeyBiryukov]

In 51454:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertTrue( ... > 0 ) with assertGreaterThan() to use native PHPUnit functionality.

Follow-up to [51335], [51337], [51367], [51397], [51403], [51404], [51436], [51438], [51448], [51449], [51451], [51453].

See #53363.

[SergeyBiryukov]

In 51461:

Tests: Use more appropriate assertions in various tests.

This replaces instances of assertFalse( stripos( ... ) ) with assertStringNotContainsString() or assertStringNotContainsStringIgnoringCase() to use native PHPUnit functionality.

Going forward, these methods introduced in PHPUnit 7.5 should be used for similar assertions:

• assertStringContainsString()
• assertStringContainsStringIgnoringCase()
• assertStringNotContainsString()
• assertStringNotContainsStringIgnoringCase()

As WordPress currently uses PHPUnit 5.7.x to run tests on PHP 5.6, polyfills for these methods are now added to the WP_UnitTestCase class for PHPUnit < 7.5.

Follow-up to [51335], [51337], [51367], [51397], [51403], [51404], [51436], [51438], [51448], [51449], [51451], [51453], [51454].

See #53363.

[SergeyBiryukov]

In 51462:

Tests: Replace assertContains() with assertStringContainsString() when used with strings.

Using the assertContains() and assertNotContains() methods with string haystacks was deprecated in PHPUnit 8 and removed in PHPUnit 9.

While WordPress test suite currently only supports PHPUnit up to 7.5.x, this allows us to switch to newer assertions ahead of adding full support for PHPUnit 8+.

These methods introduced in PHPUnit 7.5 should be used as an alternative:

• assertStringContainsString()
• assertStringContainsStringIgnoringCase
• assertStringNotContainsString()
• assertStringNotContainsStringIgnoringCase

As WordPress currently uses PHPUnit 5.7.x to run tests on PHP 5.6, polyfills for these methods were added to the WP_UnitTestCase class for PHPUnit < 7.5.

Follow-up to [51331], [51451], [51461].

Props jrf, dd32, SergeyBiryukov.
See #53363, #46149.

[jrf]

Annotate when tests do deliberately do not perform assertions, so the test won't be marked as risky

[johnbillion]

Related: #41463

[jrf]

Use more appropriate assertion [1] - this assertion is available PHPUnit cross-version, so can already be used

[jrf]

Use more appropriate assertion [2] - this assertion is available PHPUnit cross-version, so can already be used

[jrf]

Note: both methods used in the above two patches were introduced in PHPUnit 5.6.0. As the test runs on PHPUnit 5.x use PHPUnit 5.7.27, this shouldn't be an issue, though the test bootstrap.php file does give the impression that PHPUnit 5.4 is still supported (though I don't understand why we'd want to support anything but the last version of the PHPUnit 5.x series).

[jrf]

A concrete test class should be suffixed with Test, not UnitTestCase(s)

[jrf]

Build/Tests: custom assertions should always have a $message parameter All assertions in PHPUnit have a $message parameter. Setting this parameter allows to distinguish which assertion is failing when a test runs multiple assertion, making debugging of the tests easier. The WP custom assertions which are included in the WP_UnitTestCase_Base class should also allow for this $message parameter. This patch adds this - optional - parameter for those assertions which were missing it.

[jrf]

Putting a hold on patch 53363-01.patch /cc @johnbillion

There is something weird going on with this.

• If the tests are run with WP_RUN_CORE_TESTS set to 0, those tests are marked as "risky" due to not performing any assertions. The patch would fix that.
• However, if the tests are run with WP_RUN_CORE_TESTS set to 1 + this patch applied, those same tests are marked as "risky" saying that 4 assertions were run - even though the tests are empty.

This leads me to believe that somewhere assertions are being used within a fixture method (setUp() et al). This is bad practice and should not be allowed.

I've not dug in any deeper yet, but I do believe that this underlying issue needs to be solved first before this patch can be accepted.

[johnbillion]

Yes there are definitely non-test methods in the test suite that perform assertions. I'll try to dig them out.

[jrf]

@johnbillion Appreciated! Thanks.

[SergeyBiryukov]

Replying to jrf:

Note: both methods used in the above two patches were introduced in PHPUnit 5.6.0. As the test runs on PHPUnit 5.x use PHPUnit 5.7.27, this shouldn't be an issue, though the test bootstrap.php file does give the impression that PHPUnit 5.4 is still supported (though I don't understand why we'd want to support anything but the last version of the PHPUnit 5.x series).

Right, WordPress test suite still supports PHPUnit 5.4.x as the minimum version as of [47880].

That said, I don't see any issue with bumping that requirement to PHPUnit 5.7.x.

[johnbillion]

Here's a few I found from a quick listener I knocked up. There's likely more.

• Tests_WP_Customize_Widgets::setUp(): ​https://github.com/WordPress/wordpress-develop/blob/92c454cec6c214c29ca54859c79d47a8f7d9fa68/tests/phpunit/tests/customize/widgets.php#L67-L73
• Tests_AdminBar::get_standard_admin_bar(): ​https://github.com/WordPress/wordpress-develop/blob/92c454cec6c214c29ca54859c79d47a8f7d9fa68/tests/phpunit/tests/adminbar.php#L247-L248
• WP_Test_REST_Controller_Testcase::filter_rest_url_for_leading_slash(): ​https://github.com/WordPress/wordpress-develop/blob/92c454cec6c214c29ca54859c79d47a8f7d9fa68/tests/phpunit/includes/testcase-rest-controller.php#L48
• Tests_User_Session::setUp(): ​https://github.com/WordPress/wordpress-develop/blob/92c454cec6c214c29ca54859c79d47a8f7d9fa68/tests/phpunit/tests/user/session.php#L15-L16
• WP_Test_XML_TestCase::loadXML(): ​https://github.com/WordPress/wordpress-develop/blob/92c454cec6c214c29ca54859c79d47a8f7d9fa68/tests/phpunit/includes/testcase-xml.php#L22-L25
• WP_Import_UnitTestCase::_import_wp(): ​https://github.com/WordPress/wordpress-develop/blob/92c454cec6c214c29ca54859c79d47a8f7d9fa68/tests/phpunit/tests/import/base.php#L27-L28

[jrf]

WP_Test_REST_Controller_Testcase::filter_rest_url_for_leading_slash() would seem the most likely culprit.

The function is added as a callback to a filter in the (parent) WP_Test_REST_Controller_Testcase::setUp() and the rest_api_init action is also called during setUp() - I still have to trace whether that causes the filter_rest_url_for_leading_slash() method to run, but seems likely, though that one filter callback being called four times during setUp() does feel like there may be an underlying bug in the REST API Controller logic anyway.

While looking at this, I also noticed another issue with the setUp() and tearDown(): setUp() adds the filter_rest_url_for_leading_slash() method as a callback, but tearDown() tries to remove test_rest_url_for_leading_slash() as a callback.
Not a real problem as the WP_UnitTestCase_Base::tearDown() calls WP_UnitTestCase_Base::_restore_hooks() which reset the hooks queue to its original state, but just thought I'd mention it as it does seem off.

[jrf]

Confirmed: commenting out the assertion in the filter_rest_url_for_leading_slash() method removes the "This test is annotated with "@doesNotPerformAssertions" but performed 4 assertions" risky warning.

Still have to dig in deeper to figure out where the callback is triggered and what should be done with it.

[johnbillion]

One more:

• Tests_DB_Charset::data_strip_invalid_text() (assertions inside a data provider, nice): ​https://github.com/WordPress/wordpress-develop/blob/92c454cec6c214c29ca54859c79d47a8f7d9fa68/tests/phpunit/tests/db/charset.php#L418-L419

[jrf]

Replying to SergeyBiryukov:

Replying to jrf:
Right, WordPress test suite still supports PHPUnit 5.4.x as the minimum version as of [47880].

That said, I don't see any issue with bumping that requirement to PHPUnit 5.7.x.

@SergeyBiryukov FYI: The requirement will be raised to 5.7.21 in the polyfills patch anyway, so we could just wait a little for that patch to land before those two assertion change patches get committed ?

(assertions inside a data provider, nice)

@johnbillion Ouch... nice find though. I think we should aim to fix all of these in the foreseeable future.

[SergeyBiryukov]

In 51476:

Tests: Correct class name for WP_Filesystem_Base::find_folder() tests.

A concrete test class should be suffixed with Test, not UnitTestCase(s).

Follow-up to [25053].

Props jrf.
See #53363.

[SergeyBiryukov]

In 51478:

Tests: Add a $message parameter for custom assertions in WP_UnitTestCase_Base.

All assertions in PHPUnit have a $message parameter. Setting this parameter allows to distinguish which assertion is failing when a test runs multiple assertions, making debugging of the tests easier.

This optional parameter is now added for the assertion methods in the WP_UnitTestCase_Base class that were missing it.

Props jrf.
See #53363.

[SergeyBiryukov]

In 51479:

Tests: Correct placement of the $message parameter in assertDiscardWhitespace().

Follow-up to [51478].

Props johnbillion.
See #53363.

[SergeyBiryukov]

In 51480:

Tests: Modernize the WP_UnitTestCase_Base::assertEqualFields() method:

• Use assertSame() instead of fail() to display a proper message in case of failure.
• Add an optional $message parameter for consistency with other assertions.

Follow-up to [51478], [51479].

See #53363.

[SergeyBiryukov]

In 51481:

Tests: Use better assertions in WP_UnitTestCase_Base::assertEqualFields():

• Check if the object attribute exists before checking its value.
• Mention the field name in error messages in case of failure.

Follow-up to [51478], [51479], [51480].

Props jrf.
See #53363.

[jrf]

Found something else which needs looking into, where the tests clearly aren't self-contained:

If I add the import group to the <groups> <excludes>, then the following two unrelated tests start erroring out:

#) Tests_Embed_Template::test_oembed_output_post_with_thumbnail
DOMDocument::loadHTML(): AttValue: " expected in Entity, line: 10

path/to/wp/tests/phpunit/tests/oembed/template.php:66

#) Tests_Embed_Template::test_oembed_output_attachment
DOMDocument::loadHTML(): AttValue: " expected in Entity, line: 10

path/to/wp/tests/phpunit/tests/oembed/template.php:110

[johnbillion]

@jrf Thanks, can you open a separate ticket for that please?

### QA: move incorrectly placed test file

The Block_Supported_Styles_Test class is not a TestCase to be extended, but an actual concrete test class.

Note: the tests as-is are failing. This should be fixed before this can be merged.

Includes minor CS fixes and adding the $message parameter to assertions to aid in debugging.

### Block_Supported_Styles_Test: update the style related test to current expected format

### Block_Supported_Styles_Test: remove two outdated tests

The functionality which these tests were testing is no longer available in this manner, so these tests are redundant.

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

[jrf]

I've just opened ​PR 1519 which is a collaborative effort between @aristath and me with some consulting from @youknowriad to fix a misplaced test.

The test was in the phpunit/includes directory, meaning it was never actually run.
When fixing the location though, turned out the tests were failing.
All fixed now and ready for review/commit.

[jrf]

Replying to johnbillion:

@jrf Thanks, can you open a separate ticket for that please?

Done: https://core.trac.wordpress.org/ticket/53781

[jrf]

The three commits of GH PR 1519 as one patch ready for commit

[SergeyBiryukov]

In 51490:

Tests: Move and fix incorrectly placed tests for block supported styles.

The Block_Supported_Styles_Test class is not a TestCase to be extended, but an actual concrete test class. In order to run as expected, it should be placed under phpunit/tests/blocks/ along with the other block tests.

Additionally:

• Add missing visibility keywords to test methods.
• Update the expected results to the currently used format for the tests to pass.
• Remove two outdated tests. The functionality being tested there is no longer available in this manner, so these tests are redundant.

Follow-up to [49226], [49310].

Props jrf, aristath, youknowriad.
See #53363.

Closing as committed via https://core.trac.wordpress.org/changeset/51490

[SergeyBiryukov]

In 51491:

Tests: Rename classes in phpunit/tests/blocks/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969].

See #53363.

[SergeyBiryukov]

In 51492:

Tests: Rename classes in phpunit/tests/sitemaps/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491].

See #53363.

[SergeyBiryukov]

In 51493:

Tests: Rename classes in phpunit/tests/widgets/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491], [51492].

See #53363.

[jrf]

Replying to jrf:

Replying to SergeyBiryukov:

Replying to jrf:
Right, WordPress test suite still supports PHPUnit 5.4.x as the minimum version as of [47880].

That said, I don't see any issue with bumping that requirement to PHPUnit 5.7.x.

@SergeyBiryukov FYI: The requirement will be raised to 5.7.21 in the polyfills patch anyway, so we could just wait a little for that patch to land before those two assertion change patches get committed ?

Both the 53363-02-use-assertfileisreadable.patch and the 53363-03-use-assertdirectoryexists.patch changes are now unblocked after the commits made for #47381, which raised the minimum PHPUnit version to 5.7.x as the assertion functions being introduced in those patches are available since PHPUnit 5.6.0.

[jrf]

Build/Tests: fix message display in test bootstrap Any messages to the user which are echo-ed out in the test bootstrap will generally display on a command-line interface. The *nix specific "\n" line ending will be ignored on Windows, making the messages less readable. For new lines in CLI messages, PHP_EOL should be used instead. This was already done in a few places in the script, but not consistently so. Fixed now.

[jrf]

PHPUnit config files: add schema reference The current config files validate against the PHPUnit XSD schema for config files for PHPUnit 5.7 - 9.2. The schema was changes in PHPUnit 9.3 and the filter and logging configuration deprecated in favour of coverage and a different format for logging. By setting the schema, deprecation notices about the use of an outdated schema when running on PHPUnit 9.3+ are prevented.

[jrf]

Build/Tests: testcases should be abstract TestCases which are intended to be extended and not run directly, should be abstract.

[SergeyBiryukov]

In 51579:

Tests: Use more appropriate assertions in get_themes() tests.

This replaces instances of assertTrue( is_file( ... ) ) followed by assertTrue( is_readable( ... ) ) with assertFileIsReadable() to use native PHPUnit functionality.

The assertFileIsReadable() method was introduced in PHPUnit 5.6. As the minimum supported PHPUnit version has been raised to PHPUnit 5.7.21, it can now be used.

Follow-up to [51543], [51574].

Props jrf.
See #53363.

[SergeyBiryukov]

In 51580:

Tests: Use more appropriate assertions in get_themes() tests.

This replaces instances of assertTrue( is_dir( ... ) ) with assertDirectoryExists() to use native PHPUnit functionality.

The assertDirectoryExists() method was introduced in PHPUnit 5.6. As the minimum supported PHPUnit version has been raised to PHPUnit 5.7.21, it can now be used.

Follow-up to [51543], [51574], [51579].

Props jrf.
See #53363.

[SergeyBiryukov]

In 51581:

Build/Test Tools: Fix message display in test bootstrap.

Any messages to the user which are echo-ed out in the test bootstrap will generally display on a command-line interface.

The *nix specific "\n" line ending will be ignored on Windows, making the messages less readable.

For new lines in CLI messages, PHP_EOL should be used instead.

This was already done in a few places in the script, but not consistently so. Fixed now.

Follow-up to ​[UT882], ​[UT890], [44723], [45020], [48592], [49535], [51560].

Props jrf.
See #53363.

[jrf]

Tests/bootstrap: fix unicode char having slipped in Follow up on [51581]

[SergeyBiryukov]

Just noting that I was confused by 53363-07-add-schema-to-config.patch​ at first, as I still see a deprecated schema warning in PHPUnit 9.3+ with this patch, both locally and ​on GitHub:

Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

It looks like either this message or a more detailed one:

Warning - The configuration file did not pass validation!
The following problems have been detected:
...
Test results may not be as expected.

is displayed in TestRunner::run(), depending on ​whether a schema is detected.

Per discussion with @jrf, the first warning will remain and is fine. Only when PHPUnit 10 comes into play we should look into adding a run step to CI to call the --migrate-configuration command. The patch is supposed to silence the second, more detailed warning, displayed in PHPUnit 9.3+ when an old configuration file is used.

Looking at the SchemaDetector class, it attempts to ​validate the configuration file against the ​available schema files, as per the ​SchemaFinder class.

In my testing with PHPUnit 9.5.8, the configuration file successfully validates against the PHPUnit 9.2 schema with or without it being explicitly declared, so the detailed message is never displayed and the patch does not seem to make a difference. Still, I guess it's better to declare the schema explicitly for clarity.

[SergeyBiryukov]

In 51583:

Build/Test Tools: Add schema reference to PHPUnit config files.

The current config files validate against the PHPUnit XSD schema for config files for PHPUnit 5.7 – 9.2.

The schema was changed in PHPUnit 9.3, and the filter and logging settings were deprecated in favor of coverage and a different format for logging.

This commit explicitly sets the schema against which the files currently validate, for clarity.

Props jrf.
See #53363.

[SergeyBiryukov]

In 51584:

Build/Test Tools: Remove Unicode character from PHPUnit version check message.

Not all CLI tools can handle Unicode characters or non-system specific line endings well, so this type of CLI messaging should always be written with the optimal cross-platform, cross-CLI tool end-user experience in mind.

Follow-up to [51581].

Props jrf.
See #53363.

[SergeyBiryukov]

In 51585:

Build/Test Tools: Declare two TestCase classes as abstract.

TestCases which are intended to be extended and not run directly, should be abstract.

Follow-up to [763/tests], [30277].

Props jrf.
See #53363.

[SergeyBiryukov]

In 51623:

Tests: Rename classes in phpunit/tests/formatting/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491], [51492], [51493].

See #53363.

[SergeyBiryukov]

In 51628:

Tests: Move loading the PO class to set_up_before_class().

This ensures that the class is loaded once before the first test of the test case class is run, and require_once() is not unnecessarily called for each test method individually.

Follow-up to [1106/tests].

See #53363.

[SergeyBiryukov]

In 51639:

Tests: Rename classes in phpunit/tests/admin/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491], [51492], [51493], [51623].

See #53363.

[SergeyBiryukov]

In 51641:

Tests: Move loading the WP_Community_Events class to set_up_before_class().

This ensures that the class is loaded once before the first test of the test case class is run, and require_once() is not unnecessarily called for each test method individually.

Follow-up to [40607], [51628].

See #53363.

[SergeyBiryukov]

In 51646:

Tests: Rename classes in phpunit/tests/editor/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491], [51492], [51493], [51623], [51639].

See #53363.

[SergeyBiryukov]

In 51650:

Tests: Rename classes in phpunit/tests/error-protection/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491], [51492], [51493], [51623], [51639], [51646].

See #53363.

[SergeyBiryukov]

In 51651:

Tests: Rename classes in phpunit/tests/feed/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491], [51492], [51493], [51623], [51639], [51646], [51650].

See #53363.

[SergeyBiryukov]

In 51663:

Tests: Move wp_list_pluck() tests to their own file.

The tests were partially duplicated in two separate files.

Follow-up to [431/tests], [28900], [38928], [42527].

See #53363, #53987.

[SergeyBiryukov]

In 51664:

Tests: Remove duplicate wp_list_pluck() tests.

The tests were partially duplicated in two separate files, and are now located in their own file.

Follow-up to [431/tests], [28900], [38928], [42527], [51663].

See #53363, #53987.

[SergeyBiryukov]

In 51665:

Tests: Rename the test file and class for wp_filter_object_list() tests.

This matches the name of the function being tested.

Follow-up to [410/tests], [51663], [51664].

See #53363, #53987.

[jrf]

Tests bootstrap: improve an outdated error condition The PHPUnit tests are/should generally be run on the src directory, so changes just made can be tested. While testing via the build directory is also still supported, this is not the default. This was last changed in [50441], but that commit did not remove/update the error message thrown by the test bootstrap file. This last change also did not take into account that that change meant that people had to update their own wptests-config.php file to match the change made in Core and would otherwise still get the outdated error message. This commit changes the messaging in the test bootstrap file to be more in line with the current reality, while still accounting for the fact that tests can be run from both src as well as build`.

[SergeyBiryukov]

In 51666:

Tests: Move wp_list_filter() tests to their own file.

This matches the name of the function being tested.

Follow-up to [38928], [51663-51665].

See #53363, #53987.

[SergeyBiryukov]

In 51667:

Tests: Move wp_list_sort() tests to their own file.

This matches the name of the function being tested.

Follow-up to [38928], [51663-51666].

See #53363, #53987.

[SergeyBiryukov]

In 51668:

Coding Standards: Add missing visibility keywords for wp_filter_object_list() and wp_list_pluck() tests.

Follow-up to [51663-51667].

Props pbearne.
See #53363, #53987.

[hellofromTonya]

@jrf and I discussed 53363-09-test-bootstrap-improve-error-msg.patch​ per notes she added to the patch attachment. I tested before and after with each scenario. Works as expected.

Test Report

Env:

• WordPress: trunk
• OS: macOS Big Sur
• localhost: npm / Docker

Setup:

npm install
npm run build:dev
npm run env:start
npm run env:install
composer install

Scenario 1: ABSPATH is build but build folder does not exist

Steps:

• Change the ABSPATH to build in the wp-tests-config.php file:

  define( 'ABSPATH', dirname( __FILE__ ) . '/build/' );
  

• Run the tests (here running one group of tests but could be any group or all tests)

  npm run test:php -- --group formatting
  

Results:
Before patch:

Error: The /build/ directory is missing! Please run `npm run build` prior to running PHPUnit.

After applying patch:

Error: The PHPUnit tests should be run on the /src/ directory, not the /build/ directory. Please update the ABSPATH constant in your `wp-tests-config.php` file to `dirname( __FILE__ ) . '/src/'` or run `npm run build` prior to running PHPUnit.

Works as expected ✅

Scenario 2: ABSPATH is not src or build and the folder does not exist

Steps:

• Change the ABSPATH to doesnotexist in the wp-tests-config.php file:

  define( 'ABSPATH', dirname( __FILE__ ) . '/doesnotexist/' );
  

• Run the tests (here running one group of tests but could be any group or all tests)

  npm run test:php -- --group formatting
  

Results:
Before patch:

Error: The /build/ directory is missing! Please run `npm run build` prior to running PHPUnit.

After applying patch:

Error: The ABSPATH constant in the `wp-tests-config.php` file is set to a non-existent path "/var/www/doesnotexist/". Please verify.

Works as expected ✅

Scenario 3: ABSPATH is build and build folder exists

Steps:

• In command-line, run: npm run build
• Change the ABSPATH to build in the wp-tests-config.php file:

  define( 'ABSPATH', dirname( __FILE__ ) . '/build/' );
  

• Run the tests (here running one group of tests but could be any group or all tests)

  npm run test:php -- --group formatting
  

Results:
No errors. Tests run. Works as expected ✅

[hellofromTonya]

Whoopsie, I forgot to add the ticket number to changeset [51669]. Commenting here to (manually) link the changeset to this ticket.

In 51669:
Tests: Improve bootstrap error message for when ABSPATH folder does not exist.

The PHPUnit tests are/should generally be run on the src directory, so changes just made can be tested. While testing via the build directory is also still supported, this is not the default.

This was last changed in [50441], but that commit did not remove/update the error message thrown by the test bootstrap file.

This last change also did not take into account that that change meant that people had to update their own wptests-config.php` file to match the change made in Core and would otherwise still get the outdated error message.

This commit changes the messaging in the test bootstrap file to be more in line with the current reality, while still accounting for the fact that tests can be run from both src as well as build.

Follow-up to [49569], [50441], [51581].

Props jrf, hellofromTonya.

[desrosj]

In 51670:

Build/Test Tools: Remove shell: bash from PHPUnit test workflow.

bash is the default shell for all non-Windows GitHub Action runners, so this is not necessary.

See #53363.

[desrosj]

In 51671:

Build/Test Tools: Remove shell: bash from code coverage workflow.

bash is the default shell for all non-Windows GitHub Action runners, so this is not necessary.

Follow up to [51670].
See #53363.

[jrf]

WP_UnitTestCase_Base: more improvements to custom assertions The assertEqualFields() method expects an object and a fields array as inputs and subsequently approaches the received parameters as such, but did not verify whether the received parameters are of the expected types. Along the same lines, the assertSameSets(), assertEqualSets(), assertSameSetsWithIndex() and the assertEqualSetsWithIndex() methods all expect arrays for both the actual as well as the expected values and uses the array function [k]sort() on both, but never verified that the received inputs were actually arrays, which could lead to PHP errors on the sorting function calls.

[jrf]

WP_UnitTestCase_Base::assertDiscardWhitespace(): stabilize the custom assertion The assertDiscardWhitespace() method uses assertEquals() under the hood, meaning that in reality any type of actual/expected value should be accepted by the function. Fixed the documentation to reflect that. At the same time, only strings can contain whitespace differences, so the whitespace replacement should only be done when string values are passed. This will prevent potential passing null to non-nullable errors on PHP 8.1, if either of the inputs would turn out to be null.

[jrf]

Uploaded two more patches for the custom assertions. Both are ready for review.

[hellofromTonya]

In 51697:

Tests: Test custom assertions parameter data type in WP_UnitTestCase_Base.

The following changes improve tests stability.

The assertEqualFields() method expects an object and a fields array as inputs and subsequently approaches the received parameters as such, but did not verify whether the received parameters are of the expected types.

Along the same lines, the assertSameSets(), assertEqualSets(), assertSameSetsWithIndex() and the assertEqualSetsWithIndex() methods all expect arrays for both the actual as well as the expected values and uses the array function [k]sort() on both, but never verified that the received inputs were actually arrays, which could lead to PHP errors on the sorting function calls.

Follow-up to [30687], [42343], [48937], [48939], [51480], [51481].

Props jrf.
See #53363.

[hellofromTonya]

In 51698:

Tests: Do whitespace replacement in assertDiscardWhitespace() only when string.

The assertDiscardWhitespace() method uses assertEquals() under the hood, meaning that in reality any type of actual/expected value should be accepted by the function. Fixed the documentation to reflect that.

At the same time, only strings can contain whitespace differences. So the whitespace replacement should only be done when string values are passed.

This change (a) prevents potential passing null to non-nullable errors on PHP 8.1, if either of the inputs would turn out to be null and (b) increases tests stability.

Follow-up to [35003], [44902].

Props jrf.
See #53363.

[hellofromTonya]

In 51792:

Tests: Add more invalid IP test cases and @covers to Tests_Functions_Anonymization.

Adds a few more invalid IP test cases.

Adds extra @covers tag for the two functions which are testing the wp_privacy_anonymize_ip() function.

(At class level, the wp_privacy_anonymize_data() is set as covered).

Follow-up to [42971].

Props jrf, hellofromTonya.
See #53363.

[hellofromTonya]

In 51798:

Tests: Add tests for wpdb::_real_escape().

Adds a new dedicated test file.

Adds a test to check that various input types passed to wpdb::_real_escape() are handled correctly.

Note: This new test does not test the actual escaping or other logic in the function. Rather, it just and only tests and documents how the function handles various input types.

Props jrf, hellofromTonya.
See #53363.

[hellofromTonya]

Whoopsie attached changeset [51817] to the wrong ticket. It belongs on this ticket.

In 51817:
Build/Test Tools: Reworks Tests_Option_Option::test_bad_option_names() into a data provider.

The existing tests were running multiple functions through a foreach(). If any test failed, it would bail out and not test against the other scenarios.

This commit:

• Moves the scenarios to a data provider with named data sets, i.e. to ensure all scenarios are run and tested regardless if any fail.
• Splits each function under test into individual test methods.
• Adds a float scenario.
• Adds method visibility modifiers.

Follow-up to [25002].

Props jrf, hellofromTonya, pbearne.
See #53635.

[hellofromTonya]

In 51831:

Build/Test Tools: Fix null handling and string type casting in WP_UnitTestCase_Base::assertSameIgnoreEOL().

Basically, the whole assertSameIgnoreEOL() assertion was fundamentally flawed. The assertion contends that it checks that the expected and actual values are of the same type and value, but the reality was very different.

• The function uses map_deep() to potentially handle all sorts of inputs.
• map_deep() handles arrays and objects with special casing, but will call the callback on everything else without further distinction.
• The callback used passes the expected/actual value on to the str_replace() function to remove potential new line differences.
• And the str_replace() function will - with a non-array input for the $subject - always return a string.
• The output of these calls to map_deep() will therefore have "normalized" _all properties_ in objects, _all values_ in arrays and _all non-object, non-array values_ to strings.
• And a call to assertSame() will therefore NEVER do a proper type check as the type of all input has already, unintentionally, been "normalized" to string.

Aside from this clear flaw in the design of the assertion, PHP 8.1 now exposes a further issue as a null value for an object property, an array value or a plain value, will now yield a str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated notice.

To fix both these issues, the fix in this PR ensures that the call to str_replace() will now only be made if the input is a text string.
All other values passed to the callback are left in their original type.

This ensures that a proper value AND type comparison can be done as well as prevents the PHP 8.1 deprecation notices.

Ref:

• ​https://developer.wordpress.org/reference/functions/map_deep/
• ​https://www.php.net/manual/en/function.str-replace.php

This commit:

• Fixes type-casting of non-string values to string (the flawed part of this assertion) by invoking str_replace() when the value is of string type.
• Fixes the PHP 8.1 str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated deprecation notice.
• Micro-optimization: skips map_deep() when actual and/or expected are null (no need to process).
• Adjusts the method documentation for both this method and the assertEqualsIgnoreEOL() alias method to document that the $expected and $actual parameters can be of any type.

Follow-up to [48937], [51135], [51478].

Props jrf, hellofromTonya.
See #53363, #53635.

[hellofromTonya]

In 51852:

Build/Test Tools: Splits and improves compat tests.

Splits the tests in the tests/phpunit/tests/compat.php file up into individual test classes for each function being tested.

Improvements to individual test cases:

• Adds @covers tags.
• Adds visibility modifiers to all methods.
• Adds function availability test.
• Where relevant, fixes the assertion parameter order.
• Data provider:
  • Where relevant, reworks a test to use a data provider.
  • Where relevant, renames data provider methods to have a more obvious link to the test it applies to.
  • Makes the data provider more readable by adding keys within the data sets.
  • Moves the data provider below its associated tests.
  • Adds/removes data sets in data providers.
• Makes the actual test code more readable by using descriptive variables and multi-line function calls.
• Adds the $message parameter to all assertions when a test method contains more than one assertion.

Specifically for the _mb_substr() tests:

• Splits the test_mb_substr_phpcore() method into two test methods based on the PHP Core test files they are emulating.
• Makes the actual test code within the test_mb_substr_phpcore_basic() method more readable by using descriptive variables and multi-line function calls.
• Splits the data used for the second part of the test_mb_substr_phpcore() function, now test_mb_substr_phpcore_input_type_handling(), off into a separate data provider with named data sets.
• Removes duplicate data sets from the data_mb_substr_phpcore_input_type_handling().
  • Why? The PHP native tests test against upper/lowercase false, true, null and some other text string single quote/double quote variations. As things were, those differentiations had been undone when the coding standards were put in place, so in effect those weren't being tested anymore. And as this is userland code, there's no point in adding these differentiations back as they will be handled the same by PHP anyway (and that is safeguarded via the PHP native tests).
• Removes the "undefined variable" and "unset variable" test cases as, while those are relevant to the C code in which PHP is written, they are not relevant for testing userland code and will behave the same as the test passing null.

Follow-to [25002], [32364], [42228], [42343], [43034], [43036], [43220], [43571], [45607], [47122], [47198], [48937], [48996], [51415], [51563], [51594].

Props jrf, hellofromTonya.
See #39265, #53363.

[SergeyBiryukov]

In 51858:

Tests: Correct the @ticket reference in wp_terms_checklist() tests.

Follow-up to [48880].

See #53363, #51137.

[SergeyBiryukov]

In 51860:

Tests: Rename classes in phpunit/tests/multisite/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491], [51492], [51493], [51623], [51639], [51646], [51650], [51651].

See #53363.

[SergeyBiryukov]

In 51869:

Tests: Remove unnecessary setUp() and tearDown() methods in multisite tests.

These were originally added in [26252] to suppress database errors on setUp() and restore on tearDown() for tests that call wpmu_create_blog(), blog factory, or installation code that attempts to clear transients.

As the multisite test coverage expanded, these methods ended up being unnecessarily copied into other test classes, where database error suppression is not required.

Follow-up to [26252], [29916], [30286], [33184], [34898], [34899], [34901], [37234], [37477], [37894], [49212], [49616], [51859].

See #53363.

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

Whilst running PHPUnit locally I noticed this message from PHPUnit 9.3:

Warning: Your XML configuration validates against a deprecated schema.
Suggestion: Migrate your XML configuration using "--migrate-configuration"!

I broke down the commits in this PR to aid code-review, one commit for each PHPUnit XML file, and additional commits for white-space cleanup.

[SergeyBiryukov]

In 51870:

Tests: Don't skip some Ajax tests on multisite, add them to the ms-excluded group instead.

Follow-up to [46693], [49835].

See #53363.

Closing per comment https://core.trac.wordpress.org/ticket/54183#comment:13

Note: I would recommend against the config file update for the PHPUnit 9.3+ schema for now (PR 1709).
The old config will work fine for the time being and works cross-version, while the PHPUnit 9.3+ config would only work for PHPUnit 9.3+.

By the time PHPUnit 10 comes out (and the old config is no longer supported in PHPUnit 10), we can start to use the "on the fly" migrate option in CI if needs be:

phpunit --migrate-configuration

[SergeyBiryukov]

In 51872:

Tests: Update the Services_JSON test for PHPUnit 9.5.10/8.5.21+.

Since PHPUnit 9.5.10 and 8.5.21, PHP deprecations are no longer converted to exceptions by default (convertDeprecationsToExceptions="true" can be configured to enable this).

Reference: ​Do not convert PHP deprecations to exceptions by default; ​PHPUnit 9.5.10 changelog.

With this change, the test for the Services_JSON compat class started failing:

There was 1 failure:

1) Tests_Compat_jsonEncodeDecode::test_json_encode_decode
Failed asserting that exception of type "PHPUnit\Framework\Error\Deprecated" is thrown.

This converts the native PHPUnit ::expectDeprecation() method call in the test to a set of individual WP-specific ::setExpectedDeprecated() method calls in order to not depend on PHPUnit behavior that is no longer the default.

Additionally, this commit includes support for catching deprecation notices from _deprecated_file() function calls to the WP_UnitTestCase_Base::expectDeprecated() method.

Follow-up to [46205], [46625], [48996], [51563], [51852], [51871].

Props jrf, netweb, SergeyBiryukov.
See #54183, #54029, #53363.

This converts the slack-notifications.yml workflow into a reusable one that can be called within other workflows.

This will eliminate the need for a separate Slack Notifications workflow to run every time a workflow run completes after a push event.

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

This converts the slack-notifications.yml workflow into a reusable one that can be called within other workflows.

This will eliminate the need for a separate Slack Notifications workflow to run every time a workflow run completes after a push event.

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

[desrosj]

In 51921:

Build/Test Tools: Modify the Slack notifications workflow to be a reusable one.

The ability to reuse workflow files within GitHub Action workflows was recently added and allows for less code duplication.

In the context of WordPress Core, this also eliminates the need for an additional “Slack Notifications” workflow to run for every completed workflow.

See #53363.

[desrosj]

In 51922:

Build/Test Tools: Adjustments as a follow up to [51921].

This adjusts the syntax for using the github-scripts action.

See #53363.

[desrosj]

In 51924:

Build/Test Tools: Pass required secrets to the Slack notifications workflow.

Secrets are not available within callable workflows by default. They must be defined within the callable workflow, and passed from the calling workflow.

Follow up to [51921-51922].

See #53363.

[desrosj]

In 51925:

Build/Test Tools: Fix syntax for passing secrets to a called workflow.

Follow up to [51923].

See #53363.

[SergeyBiryukov]

In 51928:

Tests: Add @ticket references for page_on_front canonical tests.

Follow-up to [669/tests], [849/tests], [36238], [47760].

See #53363.

[SergeyBiryukov]

Replying to desrosj:

In 51921:

Build/Test Tools: Modify the Slack notifications workflow to be a reusable one.

The ability to reuse workflow files within GitHub Action workflows was recently added and allows for less code duplication.

In the context of WordPress Core, this also eliminates the need for an additional “Slack Notifications” workflow to run for every completed workflow.

See #53363.

It looks like this works for core commits, but breaks on PRs, see ​this job for example, or any subsequent PR to the wordpress-develop repo:

Run actions/github-script@441359b1a30438de65712c2fbca0abe4816fa667
  with:
    script: const previous_runs = await github.rest.actions.listWorkflowRuns({
    owner: 'WordPress',
    repo: 'wordpress-develop',
    workflow_id: 3023044,
    branch: '1736/merge',
    per_page: 1,
    page: 2,
  });
  return previous_runs.data.workflow_runs[0].conclusion;
  
    github-token: ***
    debug: false
    user-agent: actions/github-script
    result-encoding: json
(node:1870) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
TypeError: Cannot read property 'conclusion' of undefined
    at eval (eval at callAsyncFunction (/home/runner/work/_actions/actions/github-script/441359b1a30438de65712c2fbca0abe4816fa667/dist/index.js:4942:56), <anonymous>:11:44)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async main (/home/runner/work/_actions/actions/github-script/441359b1a30438de65712c2fbca0abe4816fa667/dist/index.js:4997:20)
Error: Unhandled error: TypeError: Cannot read property 'conclusion' of undefined

[desrosj]

In 51934:

Build/Test Tools: Restore Slack notifications for older branches.

In [51921], the GitHub Actions workflows were updated to utilize the Slack notifications workflow as a callable one instead of on the workflow_run event.

This eliminated the need for an additional “Slack Notifications” workflow run for every completed workflow, but only when other workflows are updated as well. This resulted in notifications from older branches breaking, as the changes in [51921] were not backported.

Instead of backporting the needed changes now (the Slack workflow is still being polished), this commit partially restores the workflow_run event for older branches so that notifications will resume.

See #53363.

[desrosj]

In 51937:

Build/Test Tools: Use the correct workflow name in notifications on workflow_run.

When a workflow is triggered through a workflow_run event, the context is not the original workflow. The details about the original workflow are passed through the github.event context.

This also moves the conditional check controlling whether the Slack workflow is run into the calling workflows to prevent them from running for pull requests.

Follow up to [51921-51922,51924-51925,51934].

See #53363.

[SergeyBiryukov]

In 51938:

Tests: Some test improvements for clean_dirsize_cache() tests:

• Move the directory being tested to the data directory, for consistency with other test data.
• Set the svn:eol-style property to native, for consistency with other files.
• Correct the test class name in dummy.txt.

Follow-up to [51246], [51910], [51911].

See #52241, #53363.

[desrosj]

In 51952:

Build/Test Tools: Escape $ within commit messages for `$variables.

This ensures the variables are preserved in the Slack message.

Props ocean90, desrosj.
See #53363.

[desrosj]

In 51953:

Build/Test Tools: Adjust Slack notifications for scheduled and workflow_dispatch events.

This makes the needed adjustments to fix Slack notifications for scheduled and workflow_dispatch events. The data needed to send notifications for these events are stored in different locations, or need to be accessed through API requests.

Follow up to [51921], [51937].
See #53363.

[desrosj]

In 51954:

Build/Test Tools: Use correct URL for a GitHub Action workflow run.

Follow up to [51921], [51937], [51953].
Unprops desrosj.
See #53363.

[SergeyBiryukov]

In 51997:

Tests: Split WP_Posts_List_Table and WP_Comments_List_Table tests into two separate files for clarity.

These were previously combined in the includesListTable.php file. Since the tests were specific neither to the _get_list_table() function nor the parent WP_List_Table class, the naming was confusing, which should now be resolved.

Follow-up to [31730], [38854], [40297], [48151], [48521], [49190], [51993].

See #53363.

[SergeyBiryukov]

In 51999:

Tests: Clean up the $_REQUEST superglobal in WP_UnitTestCase_Base::clean_up_global_scope().

This resolves an issue where setting up $_REQUEST['post_type'] and not clearing it afterwards in Tests_Admin_IncludesScreen::setup_block_editor_test() started affecting a few WP_Comments_List_Table tests after [51997]. It also ensures a similar issue does not inadvertently happen in other tests.

Follow-up to [760/tests], [51997].

See #53363.

[desrosj]

In 52002:

Build/Test Tools: Pass workflow outcome to Slack Notifications.

When using a workflow as a callable workflow, the job status check functions do not take the called workflow into account. This has caused some failures to be incorrectly reported as successful.

This adds an input to the Slack notifications workflow for when the workflow_call event is used.

See #53363.

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

[desrosj]

In 52183:

Build/Test Tools: Update all 3rd party GitHub actions to the latest versions.

See #53363.

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

By default, GHA jobs can run for 6 hours before timing out. The timeout-minutes directive allows this to be reduced in order to protect the repository against runaway or stuck processes.

I've given each job a very generous allowance. The idea is to reduce the timeout from its default 6 hours, but not to risk the job hitting the limit.

• 20 minutes for all regular test jobs
• 120 minutes for the long-running coverage job
• 5 minutes for quick jobs like Slack notifications

Refs:

• ​https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes
• ​https://twitter.com/jaredpalmer/status/1392865882934849543

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

[hellofromTonya]

Marking ​PR 1924 ready for commit.

[johnbillion]

In 52233:

Build/Test Tools: Lower the timeout for GitHub Actions jobs so runaway or stalled processes don't risk running for the default timeout duration of six hours.

See #53363

Committed in https://core.trac.wordpress.org/changeset/52233

Merged in https://core.trac.wordpress.org/changeset/52183.

Committed to SVN in https://core.trac.wordpress.org/changeset/51921.

This corrects the order of the expected and actual parameters when used in assertions so if/when they fail the failure message is correct.

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

[johnbillion]

In 52248:

Build/Test Tools: Correct the order and naming of expected and actual values in various tests.

This corrects the order of the parameters when used in assertions so if/when they fail the failure message is correct.

See #53363

https://core.trac.wordpress.org/changeset/52248

[SergeyBiryukov]

In 52253:

Tests: Remove unexpected output in wp_dashboard_recent_drafts() tests on PHP 8.1.

This follows the approach used in other tests to let PHPUnit manage the output catching and effectively ignore the output until retrieving it later via getActualOutput().

Follow-up to [45505], [51968], [52173].

See #53635, #53363.

[jrf]

@SergeyBiryukov Wondering a little about [52253] as the expectOutputRegex() is native PHPUnit, so PHPUnit was already handling the output caching.... The expectation was just set too late in the test code.

[SergeyBiryukov]

Replying to jrf:

The expectation was just set too late in the test code.

Yeah, I was considering moving the expectOutputRegex() call instead, but in my understanding that would mean restore_previous_locale() would have to run after it and could potentially never be called in case of failure, affecting other tests.

[jrf]

Replying to SergeyBiryukov:

Replying to jrf:
Yeah, I was considering moving the expectOutputRegex() call instead, but in my understanding that would mean restore_previous_locale() would have to run after it and could potentially never be called in case of failure, affecting other tests.

Ah, I understand. However, there is a difference between assertions - which will fail the test on the first failed assertion - and these type of output expectations - which will evaluate the output after the rest of the test code has run (providing no unexpected errors were encountered) -.

So moving the expectation up to before the function call generating the output, would be the way to go in this case.

[SergeyBiryukov]

Replying to jrf:

So moving the expectation up to before the function call generating the output, would be the way to go in this case.

Ah, makes sense, thanks for the clarification! It looks like this approach could also have been used in [52173], is that correct?

[SergeyBiryukov]

In 52259:

Tests: Use a simpler approach to test the output in some tests.

Instead of ignoring the output and catching it later with getActualOutput(), we can use expectOutputRegex() directly, which evaluates the output after the rest of the test code has run, if no unexpected errors were encountered.

Follow-up to [52173], [52253].

Props jrf.
See #53635, #53363.

[SergeyBiryukov]

In 52264:

Tests: Rename classes in phpunit/tests/block-supports/ per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [52069], as a continuation of the previous changes in ​[47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491], [51492], [51493], [51623], [51639], [51646], [51650], [51651], [51860].

See #53363.

[SergeyBiryukov]

In 52265:

Tests: Rename classes in block template tests per the naming conventions.

​https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#naming-and-organization

Follow-up to [51003], as a continuation of the previous changes in ​[47780], [48911], [49327], [50291], [50292], [50342], [50452], [50453], [50456], [50967], [50968], [50969], [51491], [51492], [51493], [51623], [51639], [51646], [51650], [51651], [51860], [52264].

See #53363.

[SergeyBiryukov]

In 52266:

Tests: Replace assertEquals() with assertSame() in block template 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(), assertSameSets(), or assertSameSetsWithIndex() should generally be preferred, to make the tests more reliable.

Follow-up to [51003], [51079], [52062], [52265].

See #53364, #53363, #54335.

Several tests perform assertions conditionally or iterate dynamic arrays without ensuring they're populated. If the test is faulty and the condition never evaluates to true or the array being iterated is empty then we'll never know about it.

This adds additional assertions that cover these situations.

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

[SergeyBiryukov]

In 52310:

Tests: Fix typo in a WP_Test_REST_Posts_Controller test method name.

Follow-up to [42423].

See #53363.

[SergeyBiryukov]

In 52325:

Build/Test Tools: Remove the replace:emoji-banner-text Grunt task.

The task was previously used to ensure that /*! This file is auto-generated */ comment is not included on front end as part of the inline emoji detection script.

As the wp-emoji-loader.js script is now included via file_get_contents() and wp_print_inline_script_tag() instead of grunt-include to simplify the logic, the task does not find anything to replace and is no longer necessary.

Additionally, include a line break before the wp-emoji-loader.js script content for better line wrapping.

Follow-up to [48096], [50651], [52132].

See #44632, #44306, #53363.

[SergeyBiryukov]

In 52381:

Tests: Add an assertion to test the WP_REST_Server::add_site_logo_to_index() method.

Additionally:

• Move the test for WP_REST_Server::add_active_theme_link_to_index() to a more logical place.
• Replace assertEquals() with assertSame() in site icon test to also check the type of the value.
• Use a more consistent pattern for the tests.

Follow-up to [51241], [52080].

Props ignatggeorgiev, desrosj, SergeyBiryukov.
Fixes #53516. See #53363.

[SergeyBiryukov]

In 52382:

Tests: Mock the HTTP request response in download_url() tests.

This aims to speed up the tests and minimize unrelated failures by avoiding an unnecessary external HTTP request, while still performing the intended functionality checks.

Update similar helpers in some other tests to use more consistent terminology.

Follow-up to [37907], [46175], [51626].

See #54420, #53363.

[SergeyBiryukov]

In 52387:

Tests: Fix typo in a test method name.

Follow-up to [52261].

See #53363.

[SergeyBiryukov]

In 52388:

Tests: Fix typo in a data provider name.

Follow-up to [52261], [52387].

See #53363.

[SergeyBiryukov]

In 52391:

Tests: Use shared fixtures in block theme tests.

This removes duplicate test data and aims to avoid future confusion about which themes to use in which tests.

Follow-up to [52049], [52246], [52247], [52279].

See #53363.

[hellofromTonya]

With 5.9 RC1 tomorrow, closing this ticket. Work will continue in #54725 during the 6.0 cycle.

Thanks for the comments @jrfnl , I've pushed some changes. I've left the whitespace unadjusted for now to keep the diff clean, the whitespace can be corrected at the point of commit.

Next up, we've actually got some failures as a result of these additional assertions that need investigating.

Next up, we've actually got some failures as a result of these additional assertions that need investigating.

Nah, not test failures: test successes as making these changes has probably uncovered some bugs in the underlying code 😁