Test tool and unit test improvements for 6.1

[hellofromTonya]

Previously:

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

[SergeyBiryukov]

In 53371:

Tests: Move get_inline_data() tests to a more appropriate place.

Since this is an admin template function and the tests check for specific output with certain taxonomy parameters, placing the tests along with other tests for the functions in the same file should make them easier to find and extend than when placed between general taxonomy registration tests.

Follow-up to [52841], [53368].

See #55652.

[desrosj]

In 53427:

Build/Test Tools: Link to a specific run attempt in GitHub Action Slack notifications.

This adjusts the workflow run URLs included in Slack notifications to link to the specific attempt being reported on making it easier for someone to see the proper context of a specific notification.

Additionally, this eliminates the need for a contributor to share the reason for a failure in Slack before restarting the workflow, as the link will always be to that specificattempt. When a “fixed” notification occurs for a subsequent run attempt, the link will be to the new, successful run attempt.

See #55652.

[SergeyBiryukov]

In 53433:

Tests: Improve the assertions in recommended MySQL and MariaDB version tests.

Comparing human-readable dates instead of numeric timestamps gives identical results, but makes the message more clear in case of failure.

Follow-up to [33946], [52418], [52421], [52424].

See #55791, #55652.

[SergeyBiryukov]

In 53454:

Tests: Require the zip PHP extension in block templates export file test.

This avoids a test failure if the ZipArchive class is missing.

Additionally, this commit replaces an inline check for the ZipArchive class in personal data export file tests with a @requires annotation, for consistency with similar PHP extension requirements in other tests.

Follow-up to [44786], [51415], [52286].

See #55652.

[SergeyBiryukov]

In 53463:

Tests: Clean up test images before performing assertions in image resize tests.

This makes sure there are no leftover images in case of a test failure.

Follow-up to [161/tests], [1255/tests].

See #55652.

[SergeyBiryukov]

In 53464:

Tests: Consistently pass the $force_delete parameter to wp_delete_attachment().

Convert some wp_delete_post() calls to wp_delete_attachment() to avoid an extra function call.

See #55652.

https://core.trac.wordpress.org/ticket/55652

[SergeyBiryukov]

In 53465:

Tests: Don't overwrite image metadata in a wp_calculate_image_srcset() test for zero width.

This was unnecessarily replacing the original image metadata with a scaled version, leading to a few leftover images with the -scaled suffix in the wp-content/uploads directory.

Follow-up to [35412].

See #55652.

This is for testing an improvement to Slack notifications where workflow outcomes from pull requests with a head_ref containing trunk.

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

[desrosj]

In 53466:

Build/Test Tools: Prevent inaccurate “fixed” notifications in Slack.

When searching for previous workflow runs by branch, GitHub currently includes matches within forks.

If a contributor opens a pull request from their fork with a head_ref matching a Core branch that triggers a workflow on push event, the outcomes of the workflows associated with that pull request are currently used to determine if the previous workflow run had failed.

The result is a false “fixed” notifications when a commit immediately follows a failed workflow run from the pull request.

This adds a check to skip any workflow runs not triggered by a push event to prevent these inaccurate notifications.

Props SergeyBiryukov.
See #55652.

[SergeyBiryukov]

In 53467:

Tests: Update the URL to the documentation on GitHub Actions environment variables.

Follow-up to [49264].

See #55652.

[desrosj]

In 53468:

Build/Test Tools: Correctly confirm the previous workflow run was triggered by a push event.

This fixes the code added in [53466] to look at the correct workflow object when determining the outcome of the previous workflow run.

See #55652.

[SergeyBiryukov]

In 53478:

Tests: Move the tests for individual pluggable functions into their own directory.

This aims to make the tests more discoverable and easier to expand.

Follow-up to [50790], [53473], [53477].

See #55652.

Started this branch to debug a couple of tests (webp related) which are failing on PHP 5.6 when NOT run inside the Docker container, using a standard PHP 5.6 install.

This generally would indicate a missing @requires tag or missing skip condition.

I've done a large test refactor for that particular test class as nearly all methods were testing using foreach() loops instead of data providers, so it was impossible to see what was really failing.

While looking into this I found the following curiousities:

1. A function which was changed in WP 3.5.0 wp_save_image_file(), apparently also changed return type - from bool to array|WP_Error (BC-break anyone ?) -, but the documentation does not reflect that.
   • Does nobody use the function ?
   • Or if they do, how can it be that nobody noticed this in 25 major releases ?
2. The failing tests are in part not actually testing what they should be testing and if tested this way, there seems to be some dependency on a particular (not necessarily standard) PHP compilation. I haven't been able to track down the exact difference or rather how to detect the difference, but the result is the failing tests.

Note: the last commit is very much WIP/debugging. The commits before that are valid improvements which can be (and should be) committed anyway.

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

[SergeyBiryukov]

In 53487:

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

This matches the name of the function being tested.

Follow-up to [27550].

See #55652.

[SergeyBiryukov]

In 53488:

Tests: Re-initialize WP_Rewrite before running wp_list_authors() tests.

This ensures that the expected results use the default permalink structure, not affected by other tests.

Follow-up to [27550], [27684], [53487].

See #55652.

[SergeyBiryukov]

In 53489:

Tests: Rename classes in phpunit/tests/user/ 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], [51860], [52264], [52265].

See #55652.

[SergeyBiryukov]

In 53492:

Tests: Use more consistent wording when referring to PHP deprecation notices.

Follow-up to [51619], [51695].

See #55652.

[SergeyBiryukov]

In 53494:

Tests: Rename classes in phpunit/tests/option/ 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], [51860], [52264], [52265], [53489].

See #55652.

[SergeyBiryukov]

In 53495:

Tests: Move helper functions in Tests_Image_Functions to more appropriate places.

• Move a non-test specific helper function used by multiple tests up to the top of the class to make it more easily discoverable.
• Move a test-specific helper function used by only one test to be directly below the test using the helper function and make the link with the test explicit by adding a @see tag.

Follow-up to [1201/tests], [51415].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53497:

Tests: Remove redundant skip call in Tests_Image_Functions::get_image_editor_engine_classes().

The test bootstrap requires GD to be available, so this test skip condition will never be matched.

Also, test skipping from within a helper method, which may be used in a data provider, can lead to test runtime errors.

Follow-up to [49009], [49014], [49535], [49571], [51415].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53509:

Tests: Some improvements for REST API cache priming tests:

• Give the test methods more specific names and move them closer together.
• Correct the @covers tags.

Follow-up to [53499], [53504], [53506], [53507], [53508].

See #55593, #55652.

[SergeyBiryukov]

In 53510:

Tests: Use assertSameSets() in some newly introduced tests.

This ensures that not only the array values being compared are equal, but also that their type is the same.

Going forward, stricter type checking by using assertSameSets() or assertSameSetsWithIndex() should generally be preferred, to make the tests more reliable.

Follow-up to [48939], [51137], [51943], [53499], [53504], [53506], [53509].

See #55652.

[SergeyBiryukov]

In 53521:

Tests: Add a helper method for for creating named data providers in WP_UnitTestCase_Base.

This introduces a new test helper function which allows for turning a single-level array containing text strings into a data provider with named data sets, where the value of the data set will also be used as the name of the data set.

The function contains safeguards to ensure that it is only used with data compatible with this principle and will throw generic PHP exceptions when the data is incompatible. These type of exceptions will be displayed before the tests even start running and will stop the test run when they occur.

While generally speaking, all test cases should extend the base WP_UnitTestCase_Base class, this is still made a public static method to allow for a test, which by exception directly extends the PHPUnit base TestCase or the PHPUnit_Adapter_TestCase, to also be able to use this method.

Typical usage of this method:

public function data_provider_for_test_name() {
	$array = array(
		'value1',
		'value2',
	);

	return $this->text_array_to_dataprovider( $array );
}

The returned result will look like:

array(
	'value1' => array( 'value1' ),
	'value2' => array( 'value2' ),
)

Props jrf, hellofromTonya, adamsilverstein.
See #55652.

[SergeyBiryukov]

In 53522:

Tests: Move the filter_image_editor_output_format() helper method next to the test it's used in.

Follow-up to [53292], [53495].

See #55652.

@SergeyBiryukov Shall I rebase the PR ?

Shall I rebase the PR ?

Not sure if that's necessary, the test improvement commits still seem to apply correctly :)

[SergeyBiryukov]

In 53523:

Tests: Refactor Tests_Image_Functions::test_is_image_positive() to use a data provider.

Using a data provider has a number of advantages:

1. If the first test case fails, it won't prevent the other test cases from being tested.
2. While the assertion used in this test method does have a "failure message" (👍), the output from PHPUnit itself will already be more descriptive in case of failure when using a data provider.
3. Using named test cases in the data provider will also make the --testdox output much more descriptive and informative.

The actual cases being tested, or the test itself have not been changed.

Includes:

• Changing the conditional addition of the .ico file type to be unconditional, it was only needed for PHP < 5.3.
• Adding a @covers annotation.

Follow-up to [184/tests], [42780], [53495], [53497], [53521].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53524:

Tests: Refactor Tests_Image_Functions::test_is_image_negative() to use a data provider.

Using a data provider has a number of advantages:

1. If the first test case fails, it won't prevent the other test cases from being tested.
2. While the assertion used in this test method does have a "failure message" (👍), the output from PHPUnit itself will already be more descriptive in case of failure when using a data provider.
3. Using named test cases in the data provider will also make the --testdox output much more descriptive and informative.

The actual cases being tested, or the test itself have not been changed.

Includes:

• Adding a @covers annotation.

Follow-up to [184/tests], [53495], [53497], [53521], [53523].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53525:

Tests: Refactor Tests_Image_Functions::test_is_displayable_image_positive() to use a data provider.

Using a data provider has a number of advantages:

1. If the first test case fails, it won't prevent the other test cases from being tested.
2. While the assertion used in this test method does have a "failure message" (👍), the output from PHPUnit itself will already be more descriptive in case of failure when using a data provider.
3. Using named test cases in the data provider will also make the --testdox output much more descriptive and informative.

The actual cases being tested, or the test itself have not been changed.

Includes:

• Changing the conditional addition of the .ico file type to be unconditional, it was only needed for PHP < 5.3.
• Adding a @covers annotation.

Follow-up to [184/tests], [42780], [53495], [53497], [53521], [53523], [53524].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53526:

Tests: Refactor Tests_Image_Functions::test_is_displayable_image_negative() to use a data provider.

Using a data provider has a number of advantages:

1. If the first test case fails, it won't prevent the other test cases from being tested.
2. While the assertion used in this test method does have a "failure message" (👍), the output from PHPUnit itself will already be more descriptive in case of failure when using a data provider.
3. Using named test cases in the data provider will also make the --testdox output much more descriptive and informative.

The actual cases being tested, or the test itself have not been changed.

Includes:

• Adding a @covers annotation.

Follow-up to [184/tests], [53495], [53497], [53521], [53523], [53524], [53525].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53527:

Tests: Rename some test methods in Tests_Image_Functions for consistency.

This matches the names of the functions being tested:

• file_is_valid_image()
• file_is_displayable_image()

Follow-up to [184/tests], [53523], [53524], [53525], [53526].

See #55652.

[SergeyBiryukov]

In 53528:

Tests: Reorder is_gd_image() test methods for consistency.

This moves the test for valid types first, to match file_is_valid_image() and file_is_displayable_image() tests.

Follow-up to [48798], [53527].

See #55652.

[SergeyBiryukov]

In 53529:

Tests: Refactor Tests_Image_Functions::test_wp_save_image_file() to use a data provider.

Using a data provider has a number of advantages:

1. If the first test case fails, it won't prevent the other test cases from being tested.
2. The output from PHPUnit will be more descriptive in case of failure when using a data provider.
3. Using named test cases in the data provider will also make the --testdox output much more descriptive and informative.

The actual cases being tested, or the test itself have not been changed.

Includes:

• Adding a @covers annotation.
• Adding a skip annotation for unsupported mime types.
• Adding a failure message to each assertion.

Follow-up to [1061/tests], [53495], [53497], [53521], [53523], [53524], [53525], [53526].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53530:

Tests: Refactor Tests_Image_Functions::test_mime_overrides_filename() to use a data provider.

Using a data provider has a number of advantages:

1. If the first test case fails, it won't prevent the other test cases from being tested.
2. The output from PHPUnit will be more descriptive in case of failure when using a data provider.
3. Using named test cases in the data provider will also make the --testdox output much more descriptive and informative.

The actual cases being tested, or the test itself have not been changed.

Includes:

• Adding a @covers annotation.
• Adding a failure message to each assertion.
• Making the test method name more specific.

Follow-up to [1061/tests], [53495], [53497], [53521], [53523], [53524], [53525], [53526], [53529].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53531:

Tests: Refactor Tests_Image_Functions::test_inferred_mime_types() to use a data provider.

Using a data provider has a number of advantages:

1. If the first test case fails, it won't prevent the other test cases from being tested.
2. The output from PHPUnit will be more descriptive in case of failure when using a data provider.
3. Using named test cases in the data provider will also make the --testdox output much more descriptive and informative.

The actual cases being tested, or the test itself have not been changed.

Includes:

• Adding a @covers annotation.
• Adding a failure message to each assertion.
• Adding a skip annotation for unsupported mime types.
• Making the test method name more specific.

Follow-up to [1061/tests], [53495], [53497], [53521], [53523], [53524], [53525], [53526], [53529], [53530].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53532:

Tests: Consistently check that an image was loaded in image saving tests.

Don't unnecessarily load the image twice in test_inferred_mime_types_when_saving_an_image().

Follow-up to [1061/tests], [1151/tests], [1157/tests], [29834].

See #55652.

@SergeyBiryukov Fair enough, I'm just allergic to back-merges in PRs ;-)

[SergeyBiryukov]

In 53533:

Tests: Use consistent punctuation in failure messages in Tests_Image_Functions.

Follow-up to [53523], [53524], [53525], [53526], [53529], [53530], [53531].

See #55652.

[desrosj]

In 53534:

Build/Test Tools: Adjust Slack notifications logic to account for expected non push events.

This adjusts the logic used to determine the outcome of the previous workflow run of the current one to account for schedule and workflow_dispatch events.

In the current state, only workflows triggered by push events are examined. This is causing failures when trying to post Slack notifications for the Test Coverage workflow, and inconsistent results for workflow_dispatch events when testing older branches on a schedule.

Follow up to [53466] and [53468].
See #55652.

[SergeyBiryukov]

In 53536:

Tests: Always include the error message in assertNotWPError() and assertNotIXRError().

Previously, in case of failure, WP_UnitTestCase_Base::assertNotWPError() displayed the actual error message from the passed WP_Error object, but only if the $message parameter was empty.

This made the assertion less helpful, as the actual error message was lost in case there was a non-empty $message parameter passed to the method, as per the ​Writing PHP Tests guidelines:

All PHPUnit assertions, as well as all WordPress custom assertions, allow for a $message parameter to be passed. This message will be displayed when the assertion fails and can help immensely when debugging a test. This parameter should always be used if more than one assertion is used in a test method.

This commit ensures that the actual error message is always displayed, in addition to the passed $message parameter.

The same applies to WP_UnitTestCase_Base::assertNotIXRError().

Follow-up to [34638], [40417].

Props jrf, SergeyBiryukov.
See #55652.

[SergeyBiryukov]

In 53537:

Tests: Refactor Tests_Image_Functions::test_load_directory() to split the tests and use a data provider.

This one test was testing three different situations. When one assertion fails, the rest of the test would not be executed, so this leads to hiding one error behind another.

Splitting the test into three distinct test methods still allows for testing each situation, but tests each one in isolation and won't hide errors.

The third part of the test, dealing with image editor engine classes, will also now use a data provider.

Using a data provider has a number of advantages:

1. If the first test case fails, it won't prevent the other test cases from being tested.
2. The output from PHPUnit will be more descriptive in case of failure when using a data provider.
3. Using named test cases in the data provider will also make the --testdox output much more descriptive and informative.

The actual cases being tested, or the test itself have not been changed.

Includes:

• Adding @covers annotations.
• Adding a failure message to each assertion when multiple assertions are used in the test.
• Reusing an existing data provider with the available image editor engine classes.

Follow-up to [1061/tests], [53495], [53497], [53521], [53523], [53524], [53525], [53526], [53529], [53530], [53531].

Props jrf.
See #55652.

[SergeyBiryukov]

Note: [53537] was a follow-up to [1120/tests] rather than [1061/tests].

[SergeyBiryukov]

In 53538:

Tests: Improve Tests_Image_Functions::test_wp_crop_image*() tests.

Includes:

• Adding @covers annotations.
• Adding a failure message to each assertion when multiple assertions are used in the test.

Follow-up to [1126/tests], [53495], [53497], [53521], [53523], [53524], [53525], [53526], [53529], [53530], [53531], [53537].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53541:

Tests: Further improve Tests_Image_Functions::test_wp_crop_image*() tests.

Includes:

• Making the test method names more specific.
• Converting one-off helper methods to static closures.
• Adding a failure message to each assertion when multiple assertions are used in the test.

Follow-up to [1126/tests], [1201/tests], [53292], [53495], [53522], [53538].

See #55652.

[johnbillion]

In 53543:

I18N: Correct and improve inline docs and tests for functionality related to nooped plurals.

See #55646, #55652

[SergeyBiryukov]

In 53545:

Tests: Move assertQueryTrue() closer to the other custom assertions in WP_UnitTestCase_Base.

Follow-up to [109/tests], [26005].

See #55652.

[desrosj]

In 53553:

Build/Test Tools: Allow changes to the code coverage workflow to run on pull request.

This adds the pull_request event to the Code Coverage Report workflow, allowing changes to be verified in a pull request before being committed.

The branches and paths filters are used to limit when the workflow runs to pull requests with:

• A base branch of trunk.
• Changing specific files that can potentially affect the way a coverage report is generated.

Reports generated on pull_request events are for testing purposes only and are not submitted to Codecov.

The docker-compose.yml file has also been added to the paths filter for both push and pull_request events. Changes to this file could potentially affect the environment used to generate the report (such as the ones in [53552]).

Props afragen, johnbillion, desrosj.
See #55652.

[desrosj]

In 53554:

Build/Test Tools: Return an error when uploading a test coverage report fails.

true is now passed to the fail_ci_if_error input when the codecov/codecov-action action is used.

When uploading a code coverage report is unsuccessful, the action will now fail and return an error. This will help avoid situations like #56022 where the report was suddenly failing to upload even though the workflow itself appeared to be successful.

See #55652.

[azaozz]

In 53563:

Build/Test Tools: Fix erroneous file name, from convertInvalidEntries.php to convertInvalidEntities.php.

Props pbeane, hellofromTonya, antonvlasenko, ironprogrammer, SergeyBiryukov, costdev.
See #55652.

[SergeyBiryukov]

In 53572:

Tests: Give the tests for adding empty post meta values more consistent names.

One of these was previously renamed to mention update_metadata_by_mid().

While update_metadata_by_mid() is indeed called in wp_ajax_add_meta() to update an existing meta value, the functionality change that the test intended to verify was in the latter function.

Follow-up to [44153], [53561].

See #55652.

[SergeyBiryukov]

In 53573:

Tests: Use more consistent wording when referring to PHP deprecation notices.

Previously committed in [53492], appears to be accidentally reverted in [53564].

Follow-up to [51619], [51695], [53492], [53563].

See #55652.

[SergeyBiryukov]

In 53574:

Tests: Replace esc_url_raw() calls with sanitize_url().

Previously committed in [53455], appears to be accidentally reverted in [53562].

Follow-up to [51597], [53452], [53455], [53562].

See #39265, #55652.

[SergeyBiryukov]

Related: #56070

[SergeyBiryukov]

In 53577:

Tests: Remove multiple $wpdb::placeholder_escape() calls in wpdb tests.

This aims to improve performance of the tests by reducing the number of function calls.

Since $wpdb::placeholder_escape() saves the result in a static variable on the first run, there is no need for repeated function calls during the same request or test run, as the result would still be the same.

Follow-up to [42056].

See #55652.

[desrosj]

In 53580:

Build/Test Tools: Update NPM devDependencies to their latest versions.

This updates the following devDependencies to newer versions:

• dotenv from 16.0.0 to 16.0.1.
• grunt from 1.5.2 to 1.5.3.
• grunt-contrib-qunit from 6.0.0 to 6.2.0.
• grunt-contrib-uglify from 5.2.1 to 5.2.2.
• qunit from 2.18.2 to 2.19.1.
• sass from 1.51.0 to 1.53.0.
• sinon from 13.0.2 to 14.0.0.
• uglify-js from 3.15.4 to 3.16.1.

Additionally, npm audit fix has been run to update dependencies with vulnerabilities.

See #55652.

[desrosj]

In 53581:

Build/Test Tools: Update 3rd party GitHub Actions.

This updates the following GitHub Actions to the latest versions:

• actions/checkout
• actions/cache
• actions/github-script
• actions/setup-node
• codecov/codecov-action
• shivammathur/setup-php
• slackapi/slack-github-action

See #55652.

[desrosj]

In 53582:

Build/Test Tools: Update the actions/cache action.

This correctly updates the actions/cache action. [53581] updated the inline comment to the latest version but did not update the actual SHA value correctly.

Follow up to [53581].

See #55652.

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

[desrosj]

In 53592:

Build/Test Tools: Correct some GitHub Action workflow inline documentation.

See #55652.

[desrosj]

In 53595:

Build/Test Tools: Update 3rd party GitHub Actions.

This updates the following GitHub Actions to the latest versions:

• actions/checkout
• actions/cache
• actions/github-script
• actions/setup-node
• codecov/codecov-action
• shivammathur/setup-php
• slackapi/slack-github-action

Various inline documentation updates are also included.

Merges [53581-53582], and [53592] to the 6.0 branch.
See #55652.

[desrosj]

In 53596:

Build/Test Tools: Update 3rd party GitHub Actions.

This updates the following GitHub Actions to the latest versions:

• actions/checkout
• actions/cache
• actions/github-script
• actions/setup-node
• codecov/codecov-action
• ramsey/composer-install
• shivammathur/setup-php
• slackapi/slack-github-action

Various inline documentation updates are also included.

Merges [53112], [53581], [53582], and [53592] to the 5.9 branch.
See #55652.

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

[SergeyBiryukov]

In 53634:

Build/Test Tools: Add support for WP_Error in the test suite's wp_die() handlers.

This brings parity with WordPress core wp_die() handlers and ensures that if a WP_Error object is passed as the $message argument to wp_die(), the PHPUnit test suite displays the error message correctly.

Previously, this would cause a silent fatal error: Object of class WP_Error could not be converted to string, leading to just displaying wp_die called without any further details.

Follow-up to [28797], [41966], [44666], [45160], [47882].

See #55652.

[SergeyBiryukov]

In 53635:

Docs: Add @since tags for wp_die() handlers in the PHPUnit test suite.

Follow-up to [289/tests], [28797], [41966], [53634].

See #55652, #55646.

[SergeyBiryukov]

In 53637:

Build/Test Tools: Include the actual _doing_it_wrong() message or deprecation notice in the output.

This aims to provide better context and more details if an unexpected _doing_it_wrong() message or deprecation notice is encountered during a test run.

Previously, this would display a message like Unexpected incorrect usage notice for [...], but without any further details, making it harder to track down the actual issue.

Follow-up to [25402], [25408], [25785], [37861], [51872].

See #55652.

[SergeyBiryukov]

For reference, the discussion in ​PR #2826 was the context for [53637].

[SergeyBiryukov]

In 53638:

Docs: Add @since tags for _doing_it_wrong() and deprecation notice handlers in the PHPUnit test suite.

This affects methods in the WP_UnitTestCase_Base class:

• ::expectDeprecated()
• ::expectedDeprecated()
• ::setExpectedException()
• ::deprecated_function_run()
• ::doing_it_wrong_run()

Follow-up to [25402], [25408], [25785], [37861], [40536], [40539], [40872], [51872], [53637].

See #55652, #55646.

[azaozz]

In 53681:

Tests: Remove duplicate testcase test_get_privacy_policy_url_should_return_empty_when_privacy_policy_page_not_set() from Tests_Url_GetPrivacyPolicyUrl.

See: #55652.

[SergeyBiryukov]

In 53686:

Tests: Separate the tests in basic.php for clarity.

There were two kinds of tests in this file:

• Tests for content of some files in the root directory:
  • license.txt
  • SECURITY.md
  • package.json
• Tests for some utility functions of the test framework itself:
  • strip_ws()
  • test_mask_input_value()

The latter are now moved to their own file, utils.php.

Follow-up to [22/tests], [81/tests], [103/tests], [25240], [26940], [28064], [28480], [28493], [28523], [28631], [42381], [47403], [53683].

See #39265, #55652.

[martin.krcho]

I added some improvements to the AJAX attachments handler tests in #56203.

[desrosj]

In 53735:

Build/Test Tools: Correctly detect the first workflow run for a branch or tag.

When trying to determine the outcome of the previous run for a GitHub Action workflow, the current run is included in the list fetched from the GitHub API.

This adjusts the logic checking for the previous run to account for that and fixes notifications for the first workflow runs of a new branch or tag.

See #55652.

[desrosj]

In 53736:

Build/Test Tools: Make the GitHub Action pattern matching for tags more specific.

This improves the tag pattern matching for GitHub Action workflows to be more specific. The * wildcard in the current patterns matches any character except slash (/). While this correctly matches a version like X.Y.Z, it could also match non-numeric characters.

This changes patterns to use the + character, which matches one or more of the preceding characters ([0-9] in this case).

See #55652.

[desrosj]

In 53737:

Build/Test Tools: Add tag pattern matching for the testing NPM workflow.

This workflow was missed in [50298].

See #55652.

[desrosj]

In 53738:

Build/Test Tools: Add tag pattern matching for the testing NPM workflow.

This workflow was missed in [50298].

Merges [53737] to the 6.0 branch.
See #55652.

[desrosj]

In 53761:

Twenty Twenty-One: Update NPM dependencies.

This updates the NPM dependencies for the Twenty Twenty-One theme to the latest versions.

Modifications to the built files are included in this commit, which are a result of updating the sass dependency to the latest version.

Previously, trailing loud comments (/* ... */) were pushed to the next line,. Now the comment location is preserved, resulting in the built CSS files more closely resembling the theme’s SCSS files.

See #55652.

[desrosj]

In 53762:

Bundled Themes: Update NPM dependencies for Twenty Twenty and Twenty Nineteen.

There are no changes to any built files after updating.

See #55652.

[SergeyBiryukov]

In 53773:

Tests: Add failure messages for site icon and custom logo tests.

This makes the assertions more helpful, as per the ​Writing PHP Tests guidelines:

All PHPUnit assertions, as well as all WordPress custom assertions, allow for a $message parameter to be passed. This message will be displayed when the assertion fails and can help immensely when debugging a test. This parameter should always be used if more than one assertion is used in a test method.

Follow-up to [33181], [36905].

See #55652.

[SergeyBiryukov]

In 53774:

Tests: Declare custom-logo theme support for custom logo tests.

This addresses failures in has_custom_logo() and get_custom_logo() tests when being run separately:

1) Tests_General_Template::test_has_custom_logo
Custom logo should not be set after removal.
Failed asserting that true is false.
tests/phpunit/tests/general/template.php:291

2) Tests_General_Template::test_get_custom_logo
Custom logo should not be set after removal.
Failed asserting that a string is empty.
tests/phpunit/tests/general/template.php:336

Specifically, this ensures that the site_logo option gets deleted in _delete_site_logo_on_remove_theme_mods(), which in turn prevents the core/site-logo block filters from affecting the custom logo tests.

Alternatively, these filters could be removed instead:

remove_filter( 'theme_mod_custom_logo', '_override_custom_logo_theme_mod' );
remove_filter( 'pre_set_theme_mod_custom_logo', '_sync_custom_logo_to_site_logo' );

Follow-up to [36905], [51091], [51421], [52042].

See #55652.

[SergeyBiryukov]

In 53782:

Tests: Bring some modernization to wp_insert_post() tests.

Includes:

• Using assertIsInt() instead of assertIsNumeric() for post IDs.
• Using consistent variable names for post data and retrieved posts.
• Removing added filters before performing assertions, not after.
• Removing unused Tests_Post::$post_ids property.
• Correcting the order of arguments in some assertions.
• Converting some foreach() loops to data providers.
• Wrapping long lines for better readability.

Follow-up to [13/tests], [14/tests], [167/tests], [496/tests], [1174/tests], [1246/tests], [1287/tests], [1307/tests], [1326/tests], [30510], [33261], [33630], [34762], [46279], [50012], [51438].

See #55652.

[SergeyBiryukov]

In 53783:

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

Now that there is a separate test class for wp_publish_post() tests, some of the pre-existing tests from the general Tests_Post class can be moved there.

Follow-up to [1039/tests], [1174/tests], [46969], [49000].

See #55652.

[SergeyBiryukov]

In 53785:

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

Now that there is a separate test class for wp_insert_post() tests, some of the pre-existing tests from the general Tests_Post class can be moved there.

Includes:

• Removal of unnecessarily setting the current user to Editor for all tests.
• Removal of unnecessarily setting the cron option to an empty array for all tests.

Follow-up to [496/tests], [36607], [53782], [53783].

See #55652.

[SergeyBiryukov]

In 53787:

Tests: Don't unnecessarily set the author in some wp_insert_post() tests.

This aims to make the tests more specific. Setting the author appears to be redundant, as the as the authorship is out of scope for these particular tests.

Follow-up to [66/tests], [84/tests], [167/tests], [296/tests], [412/tests], [496/tests], [1026/tests], [1323/tests], [25554], [33041], [34762], [35183].

See #55652.

[SergeyBiryukov]

In 53788:

Tests: Don't unnecessarily randomize the post type in some wp_insert_post() tests.

Follow-up to [33041], [52389], [53785], [53787].

See #55652.

[SergeyBiryukov]

In 53789:

Tests: Use named data providers in some wp_insert_post() tests.

Follow-up to [42380], [49125], [53521], [53785], [53787], [53788].

See #55652.

[SergeyBiryukov]

In 53790:

Tests: Simplify some assertions in Tests_Media.

A number of assertions are checking for the number of times a hard-coded substring existed in a larger $haystack.

These assertions were using preg_match_all() to do so, but without actually using a regex.

In these cases, it is more performant (and simpler) to use the PHP native substr_count() function, which will yield the same result, without the overhead of regex parsing.

Reference: ​PHP Manual: substr_count()

Follow-up to [711/tests], [38838], [42694], [53558].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53795:

Tests: Simplify some function calls in Tests_Media.

The remaining assertions using preg_match_all() do actually pass a regex, but the third parameter $matches is never used.

This third parameter became optional in PHP 5.4, so we may as well remove it.

Reference: ​PHP Manual: preg_match_all()

Follow-up to [711/tests], [53558], [53790].

Props jrf.
See #55652.

[SergeyBiryukov]

In 53802:

Tests: Correct alignment in has_filter() unit test.

Follow-up to [100/tests].

See #55652, #55647.

[SergeyBiryukov]

In 53804:

Tests: Update the terminology used for action or filter names in hook tests.

This replaces the $tag variables with $hook_name, to match the core function signatures.

Follow-up to [24/tests], [62/tests], [866/tests], [1294/tests], [38571], [50807].

See #55652.

[SergeyBiryukov]

In 53805:

Tests: Update the terminology used for action or filter names in MockAction class.

This replaces the "tag" wording with "hook name" where appropriate, to match the core function signatures.

Includes:

• Introducing a ::get_hook_names() method instead of ::get_tags(), keeping the latter as an alias.
• Adding a hook_name key to the ::$events property, keeping tag for backward compatibility for now.
• Adding missing @since tags for class methods.

Follow-up to [24/tests], [62/tests], [70/tests], [50807], [53804].

See #55652.

[SergeyBiryukov]

In 53806:

Tests: Move the test for actions using closures to the general action tests file.

This was previously moved to a separate file to be excluded when running the tests on PHP 5.2.x.

Now that WordPress supports PHP 5.6.x or later, this can be moved back with the other action tests.

Follow-up to [299/tests], [301/tests], [862/tests], [866/tests], [963/tests].

See #55652.

[SergeyBiryukov]

In 53807:

Tests: Update a transients test to account for terminology changes in MockAction.

Follow-up to [33110], [53805].

See #55652.

[SergeyBiryukov]

In 53808:

Tests: Move the test for action callback representations to the general action tests file.

A separate file in the actions directory may have seemed like a good location at the time, but this is the only test left there. For consistency, it is now moved with the other action tests.

Follow-up to [1294/tests], [53806].

See #55652.

[peterwilsoncc]

In 53810:

Date/Time: Increase test coverage of wp_date().

Props costdev, pbearne, rarst.
Fixes #53485.
See #55652.

Trac ticket:

[SergeyBiryukov]

In 53824:

Tests: Simplify the list of global groups in object cache tests.

This list was not up to date due to missing blog_meta group, and does not appear to be required for the tests to pass, as WP_UnitTestCase_Base::flush_cache() adds the same list of groups, which is up to date.

Follow-up to [946/tests], [1332/tests], [40343], [42836], [53823].

See #55647, #55652.

[SergeyBiryukov]

In 53826:

Tests: Remove the list of global groups in Memcached implementation used in the test suite.

This list was not up to date, and does not appear to be required, as WP_UnitTestCase_Base::flush_cache() adds the correct list of groups, which is up to date.

Follow-up to [1332/tests], [40343], [40561], [53823], [53824].

See #55652.

[SergeyBiryukov]

In 53828:

Tests: Make the comment cache group persistent in WP_UnitTestCase_Base::flush_cache().

This brings the list of persistent groups when running the test suite in line with core.

Follow-up to [1332/tests], [37613], [52976].

See #55652.

[SergeyBiryukov]

In 53829:

Tests: Remove the list of non-persistent groups in Memcached implementation used in the test suite.

This list was not up to date, and does not appear to be required, as WP_UnitTestCase_Base::flush_cache() adds the correct list of groups, which is up to date.

Follow-up to [1332/tests], [8068], [37613], [40561], [52976], [53826], [53828].

See #55652.

[SergeyBiryukov]

In 53830:

Tests: Move wp_cache_replace() test to a more appropriate place.

This matches the location of other wp_cache_*() tests following the respective WP_Object_Cache method tests.

Follow-up to [1275/tests].

See #55652.

[SergeyBiryukov]

In 53831:

Docs: Remove obsolete comment in object cache tests.

The comment mentioned creating two cache objects with a shared cache directory, which was no longer the case.

Follow-up to [5/tests] [170/tests].

See #55646, #55652.

[SergeyBiryukov]

In 53833:

Tests: Correct data providers for get_term_link() and get_edit_term_link() tests.

• The tests use named data providers which include a flag for passing either a term ID or term object to the test, but the values for the flag were the opposite of what the array key says.
• Some array keys were duplicated, which means the earlier test case with the same name did not actually run.

Follow-up to [52180], [52255], [52258], [52305].

See #55652.

[SergeyBiryukov]

In 53836:

Tests: Combine test classes for get_edit_term_link() tests.

There were two sets of tests for the function:

• One in the link directory, based on the link-template.php file name.
• One in the term directory, based on the component name.

To avoid confusion and make it easier to decide where new tests should go in the future, the existing tests are now combined in the former location.

Includes:

• Setting the current user in ::set_up() instead of each individual test method.
• Changing the custom taxonomy name to wptests_tax for consistency with other tests.
• Moving ::register_custom_taxonomy() and ::get_term() helpers to the beginning of the class.

Follow-up to [32954], [36646], [52180], [52255], [53833].

See #55652.

[SergeyBiryukov]

In 53889:

Tests: Use named data provider for is_serialized_string() tests.

This makes the output when using the --testdox option more descriptive and is helpful when trying to debug which data set from a data provider failed the test.

Follow-up to [37357], [44478].

See #55652.

[SergeyBiryukov]

In 53890:

Tests: Bring some consistency to serialization tests.

There were two sets of tests for is_serialized():

• One in the functions.php file, based on the same file name in core.
• One in a separate class in the functions directory.

To avoid confusion and make it easier to decide where new tests should go in the future, the existing tests are now combined in the latter location.

Includes:

• Moving is_serialized() and maybe_serialize() tests into their own classes.
• Using named data providers to make test output more descriptive.
• Combining test cases and removing duplicates.

Follow-up to [278/tests], [279/tests], [328/tests], [32631], [45754], [47452], [49382], [53886], [53889].

See #55652.

[desrosj]

In 53899:

Build/Test Tools: Increase the Dependabot pull request limit for GitHub Actions.

This ensures updates for all 3rd party actions will be flagged when updates exist simultaneously.

See #55652.

[SergeyBiryukov]

In 53920:

Tests: Clean up test image before performing assertions in image tests.

This makes sure there are no leftover images in case of a test failure.

Applies to: test_wp_calculate_image_srcset_no_date_uploads().

Includes renaming the $int variable to $int_size for consistency with some other tests.

Follow-up to [34855], [35412], [35751], [53463].

See #55652.

[SergeyBiryukov]

In 53921:

Tests: Consistently skip tests for non-implemented methods in REST API test classes.

WordPress core test suite uses PHPUnit's beStrictAboutTestsThatDoNotTestAnything option set to true, which marks a test as risky when no assertions are performed.

REST API test classes have some empty tests for non-implemented methods because these test classes extend the abstract WP_Test_REST_Controller_Testcase class, which requires several methods to be implemented that don't necessarily make sense for all REST API routes.

Some of these empty tests were already marked as skipped, but not in a consistent manner. Since skipping these tests is intentional for the time being, this commit aims to bring some consistency and adjust them all to be more accurately reported as skipped instead of risky.

The skipping can be reconsidered in the future when the tests are either removed as unnecessary or updated to actually perform assertions related to their behavior.

Follow-up to [40534], [41176], [41228].

Props Mte90, tomepajk, johnbillion, zieladam, SergeyBiryukov.
See #40538, #41463, #55652.

Introduced in [38907] in response to Trac#38457.

In the set_up() method, a copy would be made of the original value of the global $wp_theme_directories variable and the intention was to restore that original value in the tear_down() method after running each test.

Unfortunately, this was not implemented correctly.

• The backup is made to a function local variable in set_up() and not stored somewhere where it is accessible from the tear_down() method.
• The restoring then references a non-existent property to restore, which would effectively set the $wp_theme_directories global to null.

Fixed by declaring a private property, storing the backup in that private property and restoring from the same.

This bug was discovered while fixing (removing) the magic methods in the WP_UnitTestCase_Base in an effort to improve compatibility with PHP 8.2.

Note: fixing the above bug _may_ cause other tests to fail if tests run _after_ this test would expect the global $wp_theme_directories variable to be null. If that's the case, I will separately submit any additional fixes needed for those tests.

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

Introduced in [38832] in response to Trac#38373.

The $editor_id property is declared as static, so can only be approached as static, even when used within the same class in which the property is declared.

Using non-static access will result in null. See: ​https://3v4l.org/93HQL

This PHP notice was hidden so far, due to the existence of magic methods in the WP_UnitTestCase_Base class.

All the same, the magic methods as they were, would also return null for this property.

All in all, the attachment being created for this test would never get the correct post_author.

Fixed by using static access to approach the static property.

This bug was discovered while fixing (removing) the magic methods in the WP_UnitTestCase_Base in an effort to improve compatibility with PHP 8.2.

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

### Tests_Rewrite_OldDateRedirect::test_old_date_redirect_cache_invalidation(): bug fix [1]

Introduced in [53549] in response to Trac#36723.

The $post_id property is declared as static, so can only be approached as static, even when used within the same class in which the property is declared.

Using non-static access will result in null. See: ​https://3v4l.org/93HQL

This PHP notice was hidden so far, due to the existence of magic methods in the WP_UnitTestCase_Base class.

All the same, the magic methods as they were, would also return null for this property.

All in all, the post being updated for this test would never get the correct post_id.

Fixed by using static access to approach the static property.

This bug was discovered while fixing (removing) the magic methods in the WP_UnitTestCase_Base in an effort to improve compatibility with PHP 8.2.

### Tests_Rewrite_OldDateRedirect::test_old_date_redirect_cache_invalidation(): bug fix [2]

Introduced in [53549] in response to Trac#36723.

The previous bug fix (using the actual $post_id instead of null) exposed that this test was in actual fact failing. This was just hidden by the first bug.

Based on the original commit introducing the test, I gather the fix I'm applying now was what the test actually _intended_ to test, but it would be good to get confirmation of this from one of the original authors. /cc @spacedmonkey

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

[jrf]

I've just opened three new PRs with bug fixes for the tests:

• ​Tests/Tests_Theme: fix bug in test set_up/tear_down
• ​WP_Test_REST_Attachments_Controller::test_update_item_parent(): bug fix
• ​Tests_Rewrite_OldDateRedirect::test_old_date_redirect_cache_invalidation(): bug fix x 2

These three are small PRs, but they are a prerequisite for the ​PHP 8.2 related PR removing the magic methods from the `WP_UnitTestCase_Base` class.

[desrosj]

In 53940:

Build/Test Tools: Update third-party GitHub Actions to latest versions.

Updated actions:

• actions/cache
• actions/github-script
• actions/setup-node
• bubkoo/welcome-action
• shivammathur/setup-php
• slackapi/slack-github-action

See #55652.

[SergeyBiryukov]

In 54040:

Tests: Correctly back up and restore theme directories in Tests_Theme.

In the set_up() method, a copy would be made of the original value of the global $wp_theme_directories variable, with the intention to restore that original value in the tear_down() method after running each test. Unfortunately, this was not implemented correctly.

• The backup is made to a function local variable in set_up() and not stored somewhere where it is accessible from the tear_down() method.
• The restoring then references a non-existent property to restore, which would effectively set the $wp_theme_directories global to null.

Fixed by declaring and using a private property to store the original $wp_theme_directories value.

This bug was discovered while fixing (removing) the magic methods in the WP_UnitTestCase_Base class in an effort to improve compatibility with PHP 8.2.

Follow-up to [38907].

Props jrf, costdev, johnbillion, swissspidy.
See #55652.

Thanks for the PR! Merged in r54040.

[SergeyBiryukov]

In 54041:

Tests: Use correct post_author value in WP_Test_REST_Attachments_Controller.

The $editor_id property is declared as static, so can only be approached as static, even when used within the same class in which the property is declared.

Using non-static access will result in null. See: ​https://3v4l.org/93HQL

This PHP notice was hidden so far, due to the existence of magic methods in the WP_UnitTestCase_Base class.

All the same, the magic methods as they were, would also return null for this property. All in all, the attachment being created for this test would never get the correct post_author.

Fixed by using static access to approach the static property.

This bug was discovered while fixing (removing) the magic methods in the WP_UnitTestCase_Base class in an effort to improve compatibility with PHP 8.2.

Follow-up to [38832].

Props jrf, costdev, johnbillion.
See #55652.

Thanks for the PR! Merged in r54041.

Thanks @SergeyBiryukov !

[SergeyBiryukov]

In 54049:

Tests: Remove @covers tags for native PHP functions in phpunit/tests/compat/.

As these are not user-defined functions, they cause notices when generating the code coverage report:

"@covers ::array_key_first" is invalid
"@covers ::array_key_last" is invalid
"@covers ::hash_hmac" is invalid
"@covers ::is_countable" is invalid
"@covers ::is_iterable" is invalid
"@covers ::mb_strlen" is invalid
"@covers ::mb_substr" is invalid
"@covers ::str_contains" is invalid
"@covers ::str_ends_with" is invalid
"@covers ::str_starts_with" is invalid

Follow-up to [51852], [52038], [52039], [52040].

See #39265, #55652.

[SergeyBiryukov]

In 54050:

Tests: Correct the @covers tags in WP::send_headers() tests for feeds.

As this is a class method and not a global function, the correct annotation syntax is @covers WP::send_headers.

Follow-up to [53233].

See #55652.

[SergeyBiryukov]

In 54051:

Tests: Correct some @covers tags in wp_html_split() and wptexturize() tests.

As preg_split() is not a user-defined function, it causes notices when generating the code coverage report:

"@covers ::preg_split" is invalid

Instead, it appears that the intention was to test the performance of these WordPress functions:

• get_html_split_regex()
• _get_wptexturize_split_regex()
• _get_wptexturize_shortcode_regex()

Follow-up to [34761], [53562].

See #39265, #55652.

[SergeyBiryukov]

In 54052:

Tests: Correct the @covers tag syntax in a taxonomy_exists() test with non-string taxonomy.

This addresses a notice when generating the code coverage report:

"@covers :taxonomy_exists" is invalid

Follow-up to [53869].

See #56338, #55652.

[SergeyBiryukov]

In 54055:

Tests: Correct the @covers tag in a WP_REST_Posts_Controller test for unique post slugs.

This addresses a notice when generating the code coverage report:

"@covers WP_REST_Request::create_item" is invalid

The WP_REST_Request class does not have a create_item() method, WP_REST_Posts_Controller is the class being tested here.

Includes fixing a typo in the test method name.

Follow-up to [53813].

See #52422, #55652.

[SergeyBiryukov]

In 54056:

Tests: Correct the @covers tag in a WP_REST_URL_Details_Controller test for registered route.

This addresses a notice when generating the code coverage report:

"@covers WP_REST_URL_Details_Controller::get_routes" is invalid

The WP_REST_URL_Details_Controller class does not have a get_routes() method, WP_REST_Server does.

Follow-up to [51973].

See #55652.

[SergeyBiryukov]

In 54057:

Tests: Correct the @covers tag in a test for strip_ws() utility function.

This addresses a notice when generating the code coverage report:

"@covers ::test_strip_ws" is invalid

Follow-up to [53686], [54049], [54050], [54051], [54052], [54055], [54056].

See #55652.

[SergeyBiryukov]

In 54058:

Tests: Explicitly mark empty REST API tests as not performing any assertions.

WordPress core test suite uses PHPUnit's beStrictAboutTestsThatDoNotTestAnything option set to true, which marks a test as risky when no assertions are performed.

REST API test classes have some empty tests for non-implemented methods because these test classes extend the abstract WP_Test_REST_Controller_Testcase class, which requires several methods to be implemented that don't necessarily make sense for all REST API routes.

As these tests are intentionally empty, they were previously marked as skipped, so that they are not reported as risky.

This commit aims to further reduce noise in the test suite and effectively ignores these empty tests altogether, which seems like a more appropriate option at this time.

The @doesNotPerformAssertions annotation can be reconsidered in the future when the tests are either removed as unnecessary or updated to actually perform assertions related to their behavior.

Follow-up to [40534], [41176], [41228], [53921].

See #40538, #41463, #55652.

[SergeyBiryukov]

In 54059:

Tests: Correct the @covers tag in a WP_REST_URL_Details_Controller test for registered route.

WP_REST_URL_Details_Controller::register_routes() appears to be a better match than WP_REST_Server::get_routes(), and is also more consistent with other test classes.

Follow-up to [51973], [54056].

See #55652.

[SergeyBiryukov]

In 54060:

Tests: Add @coversNothing tag for PHP polyfill tests in phpunit/tests/compat/.

The @covers tags for these tests were previously removed to avoid notices when generating the code coverage report on PHP versions where these functions are natively available and not user-defined:

"@covers ::array_key_first" is invalid
"@covers ::array_key_last" is invalid
"@covers ::hash_hmac" is invalid
"@covers ::is_countable" is invalid
"@covers ::is_iterable" is invalid
"@covers ::mb_strlen" is invalid
"@covers ::mb_substr" is invalid
"@covers ::str_contains" is invalid
"@covers ::str_ends_with" is invalid
"@covers ::str_starts_with" is invalid

Explicitly including a @coversNothing annotation in this case appears to be a more appropriate option than not including any annotation at all.

Follow-up to [51852], [52038], [52039], [52040], [54049].

See #39265, #55652.

[SergeyBiryukov]

In 54061:

Tests: Simplify and correct get_term_link() and get_edit_term_link() tests:

• Some assertions were unnecessarily duplicated. They aim to test the function behavior both when passing a term ID and term object, however that is already handled via the $use_id parameter of the get_term() helper in the same test class. The data providers already supply test cases both with a term ID and term object, so there is no need for a second assertion or a whole second test method with a term object.
• One get_term_feed_link() test was unnecessarily skipped half of the time, when term object was passed instead of term ID. Instead, it can use a dedicated data provider and avoid skipping.

Includes:

• Using more descriptive test method names to clarify the intention of the tests.
• Some documentation updates for clarity.

Follow-up to [52180], [52255], [52258], [52305], [53833], [53836].

See #55652.

⚠️ This PR is a work in progress and will most likely be split into multiple PRs. For now, I've just opened the PR to keep track of the current state of things. ⚠️

Trac ticket:

⚠️ This PR is a work in progress and will most likely be split into multiple PRs. For now, I've just opened the PR to keep track of the current state of things. ⚠️

Trac ticket:

[SergeyBiryukov]

In 54064:

Tests: Restore @covers tags for PHP polyfill tests in phpunit/tests/compat/.

These tags were previously removed to avoid notices when generating the code coverage report on PHP versions where these functions are natively available and not user-defined:

"@covers ::array_key_first" is invalid
"@covers ::array_key_last" is invalid
"@covers ::hash_hmac" is invalid
"@covers ::is_countable" is invalid
"@covers ::is_iterable" is invalid
"@covers ::mb_strlen" is invalid
"@covers ::mb_substr" is invalid
"@covers ::str_contains" is invalid
"@covers ::str_ends_with" is invalid
"@covers ::str_starts_with" is invalid

It has been pointed out that those tests do cover the WP implementation of those functions and should be marked as such with a @covers tag. The reason PHPUnit displays notices about it, is that code coverage is only run on PHP 7.4 instead of multiple PHP versions.

For those PHP versions which don't natively contain the function, the WP polyfill is being tested and should be seen as covered by tests. The reason the tests are also run on PHP versions in which the function already exists in PHP natively, is to make sure that the polyfill test expectations line up with the PHP native behaviour, even though at that point, they are no longer testing the WP polyfill, but the PHP native function.

With the above in mind, while those PHPUnit notices add some noise to the code coverage report, in this case, they should be ignored and the @covers tags should be brought back.

As a potential future enhancement, the code coverage script could be updated to run against the highest and lowest supported PHP versions and with some variations of extensions enabled or disabled to ensure those tests actually test the polyfills.

Follow-up to [51852], [52038], [52039], [52040], [54049], [54060].

Props jrf.
See #39265, #55652.

If the $files_that_shouldnt_exist file list would be empty, this test would be marked as risky as it wouldn't perform any assertions.

This small tweak prevents this from happening.

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

This test verifies that the WordPress readme.html file recommends a supported PHP version, however, WordPress currently still recommends PHP 7.4 due to PHP 8.0/8.1 compatibility not being fully achieved, even though PHP 7.4 is end-of-life.

As things were, the assertion in the test was commented out, leading to this test being marked as "risky" for performing any assertions.

Instead, let's skip the test with a clear skip notification.

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

[jrf]

I've opened three new PRs related to this ticket:

• ​GH PR #3181 - GH Actions vs PHP 8.1 tweaks
• ​GH PR #3182 - Makes a test non-risky
• ​GH PR #3183 - Makes a test non-risky

Reviews/merges appreciated ;-)

[SergeyBiryukov]

In 54067:

Tests: Set the current user to Editor in test_utf8mb3_post_saves_with_emoji().

This avoids a "Sorry, you are not allowed to edit this post" error further in the test. The test is currently skipped on GitHub Actions, as only runs on older MySQL versions specifically with the utf8 character set.

The user was previously set for all tests in the file in the set_up() method, however that is no longer the case, as it was not required for the majority of the tests. It is, however, necessary for the edit_post() call in this particular test.

Follow-up to [30346], [53785].

See #55652.

[SergeyBiryukov]

In 54068:

Tests: Consistently set the current user in the tests for retaining a sticky status.

This affects:

• test_user_without_publish_posts_cannot_affect_sticky()
• test_user_without_publish_posts_cannot_affect_sticky_with_edit_post()

In both tests, the user is now set after creating the post, not before. This aims to better match the intention of the tests, as they ensure that a sticky status is unaffected for a post that is edited by a user without the publish_posts capability, not necessarily created by that user.

Includes minor documentation updates for consistency.

Follow-up to [33096], [35183].

See #55652.

[SergeyBiryukov]

In 54072:

Build/Test Tools: Do not allow tests to fail for select PHP 8.1 test runs.

This affects the following test groups:

• Ajax tests
• ms-files tests
• External HTTP tests
• Xdebug tests

The tests being run in these particular test groups are passing on PHP 8.1, however, the test runs are still allowed to “continue on error”. This creates the risk that new PHP 8.1 incompatibilities will be introduced without anyone noticing.

By no longer allowing these test runs to “continue on error”, that risk is removed.

Follow-up to [51588], [51604], [53922].

Props jrf.
See #55656, #55652.

[SergeyBiryukov]

In 54073:

Tests: Prevent an Ajax test for IMAGE_EDIT_OVERWRITE from being marked as risky.

This affects Tests_Ajax_MediaEdit::testImageEditOverwriteConstant().

In case the $files_that_should_not_exist file list is empty, the test would be marked as risky, since it would not perform any assertions.

This small tweak prevents that from happening.

Follow-up to [38113].

Props jrf.
See #55652.

Thanks for the PR! Merged in r54073.

[SergeyBiryukov]

In 54074:

Tests: Temporarily skip the test for recommended PHP version in readme.html.

This test verifies that the WordPress readme.html file recommends a PHP version that is actively supported. However, WordPress currently still recommends PHP 7.4 due to PHP 8.0/8.1 compatibility not being fully achieved, even though PHP 7.4 is end-of-life.

As things were, the assertion in the test was commented out, leading to this test being marked as “risky” for not performing any assertions.

Instead, let’s skip the test with a clear skip notification.

Follow-up to [52260].

Props jrf.
See #55652.

Thanks for the PR! Merged in r54074.

[SergeyBiryukov]

In 54075:

Tests: Move Site Health unit test class to phpunit/tests/admin/.

Includes:

• Renaming the test class per the ​naming conventions.
• Creating a WP_Site_Health instance in the set_up() method, instead of leaving that to each individual test.

This brings some consistency with the tests for other admin classes, e.g. WP_Community_Events.

Follow-up to [45802], [51639].

See #55652.

[SergeyBiryukov]

In 54077:

Tests: Correct the cache invalidation tests for old date or slug redirect.

This affects:

• Tests_Rewrite_OldDateRedirect::test_old_date_redirect_cache_invalidation()
• Tests_Rewrite_OldSlugRedirect::test_old_slug_redirect_cache_invalidation()

In the former test, the $post_id property is declared as static, so can only be approached as static, even when used within the same class in which the property is declared.

Using non-static access will result in null. See: ​https://3v4l.org/93HQL

This PHP notice was hidden so far, due to the existence of magic methods in the WP_UnitTestCase_Base class.

All the same, the magic methods as they were, would also return null for this property. All in all, the post being updated for this test would never get the correct post_id.

Fixed by using static access to approach the static property.

On a related note, the described bug fix (using the actual $post_id instead of null) exposed that this test was as a matter of fact failing. This was just hidden by the first bug.

Based on the original commit introducing the test, an adjustment is now made which appears to be what the test actually intended to test. A similar change is made to the cache invalidation test for old slug redirects. While not strictly required, it brings some consistency between the two tests and ensures that both tests use a unique post_name value to avoid collisions with the previous values.

This bug was discovered while fixing (removing) the magic methods in the WP_UnitTestCase_Base class in an effort to improve compatibility with PHP 8.2.

Follow-up to [53549].

Props jrf, costdev, SergeyBiryukov.
See #55652.

Thanks for the PR! Merged in r54077, with a minor change for consistency with the related test for old slug redirects 🙂

[SergeyBiryukov]

In 54078:

Tests: Consistently create a post fixture in old date or slug redirect tests.

This affects:

• Tests_Rewrite_OldDateRedirect
• Tests_Rewrite_OldSlugRedirect

This commit updates the latter test class to create a post in the wpSetUpBeforeClass() method, for consistency with the former class. This ensures that both classes declare the $post_id property as static, to avoid a situation where non-static access is accidentally used when copying similar test cases from one class to the other.

Follow-up to [34659], [42587], [54077].

See #55652.

Merged in r54077, with a minor change for consistency with the related test for old slug redirects 🙂

Thanks @SergeyBiryukov ! Just checking: with your additional changes - is the test still testing what it should be testing ? (I haven't got time to verify at the moment, but my brain went into question mode when I looked at the adjusted commit...)

Just checking: with your additional changes - is the test still testing what it should be testing ?

I think so 🙂 My understanding of the intention if these tests is to verify that cache invalidation for _find_post_by_old_slug() and _find_post_by_old_date() functions works as expected. That is achieved by updating the post, thus changing the lookup query and the cache key.

There was a subtle difference in the test for old slug redirect, which updated the post and the cache key, but set the slug to the original value, so the permalink was not updated. In the test for old date redirect, the permalink was in fact updated, because the date was different. To avoid future confusion, I thought it would be a good idea to make these tests as consistent as possible, so they both update the permalink while clearing the cache, with the only difference that the test for old date redirect also changes the date (which it already did).

[SergeyBiryukov]

In 54088:

Tests: Correctly use the factory method.

This replaces all non-static calls to the WP_UnitTestCase_Base::factory() method with static function calls, since the method is declared as static.

This is a consistency improvement for the test suite.

Follow up to [35225], [35242], [49603], [54087].

Props jrf.
See #55652.

[SergeyBiryukov]

In 54090:

Tests: Use the factory method instead of the property.

This replaces all references to the WP_UnitTestCase_Base::$factory property with static function calls to the WP_UnitTestCase_Base::factory() method.

This is a consistency improvement for the test suite.

Follow up to [35225], [35242], [49603], [54087], [54088].

Props jrf.
See #55652.

[SergeyBiryukov]

In 54091:

Tests: Remove redundant function_exists() check in a term_is_ancestor_of() test.

The function is available as of WordPress 3.4.

Follow-up to [19678], [493/tests].

See #55652.

[desrosj]

In 54108:

Build/Test Tools: Use the default GITHUB_TOKEN instead of a personal access token.

Previously, it was not possible to use the default GITHUB_TOKEN token to create new workflow runs in an effort to prevent accidental recursive workflows.

This has changed, and the workflow_dispatch is now one of two exceptions to this rule. Using GITHUB_TOKEN is preferred whenever possible to avoid the need for a PAT (personal access token), which expires (when created using the recommended security best practices), and is tied to an individual user.

See ​https://github.blog/changelog/2022-09-08-github-actions-use-github_token-with-workflow_dispatch-and-repository_dispatch/.

See #55652.

[SergeyBiryukov]

In 54198:

Tests: Move the basic get_block_templates() test to the dedicated file.

Now that the function has its own test class, the remaining test from Tests_Block_Template_Utils can be moved to Tests_Blocks_GetBlockTemplates for consistency.

Includes:

• Uncommenting some assertions previously commented out.
• Moving the get_template_ids() helper method to the top of the class.
• Standardizing on wpSetUpBeforeClass()/wpTearDownAfterClass() in both classes.
• Declaring the test theme name as a constant in both classes, since the value is not changed by any of the tests.
• Renaming some properties in both classes for clarity.

Follow-up to [51003], [52062], [53927], [54184], [54187].

See #55652.

[SergeyBiryukov]

In 54203:

Tests: Simplify the data provider for testing whether KSES globals are defined.

To avoid duplicating array values as keys when using a named data provider, the text_array_to_dataprovider() helper method can be used.

Follow-up to [52229], [53521].

See #55652.

[desrosj]

In 54293:

Build/Test Tools: Increase the timeout value for MacOS jobs.

The current timeout-minutes value of 20 is a bit too short for MacOS jobs in GitHub Actions, which on occasion take a bit longer.

This bumps that limit to 30 to avoid unnecessarily flagging a job as stuck.

See #55652.

[desrosj]

In 54297:

Build/Test Tools: Test building WordPress to run from src first.

Because of the scripts that run when build:dev is run, it’s more common for this Grunt task to change version-controlled files than when building WordPress to run from build.

This moves the build:dev tests before the build ones in order to detect changes earlier in the workflow.

See #55652.

[SergeyBiryukov]

In 54300:

Tests: Remove empty directory in WP_UnitTestCase_Base::rmdir().

The WP_UnitTestCase_Base::rmdir() method selectively deletes files from a directory, skipping any paths from the $ignore_files property.

This commit updates the method to remove the empty directory if there are no files left, bringing some parity with PHP native rmdir() function.

Follow-up to [677/tests], [29120].

See #55652.

[SergeyBiryukov]

In 54303:

Tests: Remove nested empty directories in WP_UnitTestCase_Base::rmdir().

Includes:

• Checking if the directory exists and returning early otherwise.
• Removing a redundant rmdir() call in clean_dirsize_cache() tests.

Follow-up to [49744], [54300].

See #55652.

[SergeyBiryukov]

In 54304:

Tests: Revert removing empty directory in WP_UnitTestCase_Base::rmdir() for now.

This appears to need more investigation. Instead, delete the test-plugin and link-manager directories in REST API plugins controller tests, for which this change was initially intended.

Follow-up to [54300], [54301], [54303].

See #55652, #56629.

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

[desrosj]

In 54342:

Build/Test Tools: Update actions/github-scripts to the latest version.

This version adds support for octokit/plugin-retry.js, which retries requests automatically when 4xx or 5xx response codes are returned.

To start, the feature is configured to retry all 4xx and 5xx response codes, unless the server identifies as a teapot.

See #55652.

[desrosj]

In 54343:

Build/Test Tools: Remove the retryAfter input.

This was included in the original pull request that aimed to add support for octokit/plugin-retry.js in actions/github-scripts, but was actually removed before being merged.

Follow up to [54342].

See #55652.

[desrosj]

Just wanted to add a bit of context around a scenario where [54342] would have been useful.

If a workflow is running, it cannot be restarted. The failed workflow recently ​encountered this edge case when the dispatched workflow started running before the original one was able to clean up after itself and finish.

As it was, results of PHPCS runs would only ever show inline in a commit code view/PR code view.

This is confusing as people will often try to find an output log of the run in the GH Actions build log, not realizing the scan results are not available in the GH Actions log.

This commit changes the steps in the GH Actions workflows for both the PHPCS run against the WordPress Coding Standards as well as for the run against PHPCompatibility to show the full scan results report in the GH Actions logs _as well as_ show the results in commit/PR code views and still fail the build correctly when new issues are detected.

I've elected to store the (temporary) phpcs-report.xml file in the artifacts subdirectory as that directory is (git/svn)-ignored by default already to prevent this new, temporary file from interfering with the git diff check at the end of the workflows.

Refs:

• ​https://github.com/staabm/annotate-pull-request-from-checkstyle#using-php_codesniffer
• ​https://github.com/WordPress/gutenberg/issues/44536#issuecomment-1261753524

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

[jrf]

I've just opened PR ​GH #3368 to address confusion over where the results of PHPCS runs can be found.

Note: the PR contains two commits - only the first commit is intended to be used. The second is just there to demonstrate that the first commit works as intended.

[jrf]

​GitHub PR 3173 adds test skips for the last remaining PHP 8.1/8.2 issues to buy us time to address those later and removes the continue-on-error from the GH Actions workflows so we can actively prevent new issues being introduced.

Reviewing and committing this at your earliest convenience would be appreciated.

Can commit on Monday unless someone else beats me to it!

Can commit on Monday unless someone else beats me to it!

Sounds good to me! Thanks!

I have rebased the PR after [54364] was committed, which means the first two commits are now gone.

The last commit skipping three tests for PHP 8.1 has been replaced with a commit applying @SergeyBiryukov's suggestion.

[SergeyBiryukov]

In 54365:

Tests: Ensure prerequisites are met for draft length tests in Tests_L10n.

These three tests for wp_dashboard_recent_drafts() would run into a PHP 8.1 "passing null to non-nullable" deprecation for the call to ltrim() when the result of get_edit_post_link() is passed to esc_url().

Setting a deprecation expectation would not solve this as the returned value would still not match the expected value(s).

The recent drafts list is only displayed on the Dashboard screen for users with the edit_posts capability. By setting the current user to Editor, the prerequisites for wp_dashboard_recent_drafts() are met, which means the deprecation notice is avoided and the assertions will succeed.

This commit addresses a few errors in the test suite along the lines of:

1) Tests_L10n::test_length_of_draft_should_be_counted_by_words
ltrim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/formatting.php:4376
/var/www/src/wp-admin/includes/dashboard.php:657
/var/www/tests/phpunit/tests/l10n.php:449
/var/www/vendor/bin/phpunit:123

Follow-up to [45505], [52253], [52259].

Props jrf, desrosj, SergeyBiryukov.
See #56681, #55652, #55656.

[SergeyBiryukov]

In 54366:

Build/Test Tools: Call wpTearDownAfterClass() before deleting all data, instead of after.

As of [35186] and [51568], there are two sets of methods used for setup/teardown in the test suite before and after a test class is run:

• set_up_before_class() / tear_down_after_class()
• wpSetUpBeforeClass() / wpTearDownAfterClass(). (Note the wp prefix, these are WordPress' own methods and are not the same as the native PHPUnit setUpBeforeClass() / tearDownAfterClass() methods.)

The main difference is that wpSetUpBeforeClass() receives the $factory argument for ease of use, and both wpSetUpBeforeClass() and wpTearDownAfterClass() don't need to call self::commit_transaction().

Many tests use the wpTearDownAfterClass() method to clean up posts, users, roles, etc. created via wpSetUpBeforeClass(). However, due to how the method was previously called​, this cleanup happened after all data is already deleted from the database.

This could cause some confusion when refactoring tests. For example:

public static function wpTearDownAfterClass() {
	$GLOBALS['_wp_additional_image_sizes'] = self::$_sizes;
}

public static function tear_down_after_class() {
	wp_delete_attachment( self::$large_id, true );
	parent::tear_down_after_class();
}

At a glance, it seems like these two methods can be combined:

public static function wpTearDownAfterClass() {
	wp_delete_attachment( self::$large_id, true );

	$GLOBALS['_wp_additional_image_sizes'] = self::$_sizes;
}

However, that would not work as expected: by the time wp_delete_attachment() runs, the attachment ID is no longer in the database, so it returns early, leaving some files in the uploads directory.

By calling wpTearDownAfterClass() in WP_UnitTestCase_Base::tear_down_after_class() before deleting all data, instead of after, we ensure that both of these methods have access to the same data and can be used interchangeably to perform cleanup as necessary.

Additionally, this commit moves the calls to parent methods in WP_UnitTestCase_Base:

• parent::set_up_before_class() to be the first thing called in ::set_up_before_class()
• parent::tear_down_after_class() to be the last thing called in ::tear_down_after_class()

This does not have any effect in practice, but brings consistency with how these methods are called in the test suite.

Follow-up to [35186], [35225], [35242], [38398], [39626], [49001], [51568].

Props ironprogrammer, SergeyBiryukov.
Fixes #55918. See #55652.

Merged in r54366.

[SergeyBiryukov]

In 54369:

Build/Test Tools: Remove PHP 8.1 and 8.2 from allowed failures.

With all known unit test failures now addressed, WordPress 6.1 aims to support PHP 8.1 and 8.2 as much as possible.

While full compatibility with PHP 8.1 and 8.2 is still a work in progress, this commit aims to actively prevent new PHP issues from being introduced in WordPress core.

All remaining known issues are deprecation notices. Please note, a deprecation notice is not an error, but rather an indicator of where additional work is needed for compatibility before PHP 9 (i.e. when the notices become fatal errors). With a deprecation notice, the PHP code will continue to work and nothing is broken.

Follow-up to [49077], [49162], [50299], [51588], [51604], [53922], [54072].

Props jrf, desrosj.
See #55652, #55656, #56009, #56681.

Thanks for the PR! Merged in r54365 and r54369.

The REST API test issue was further investigated and resolved in r54368.

[desrosj]

In 54371:

Build/Test Tools: Display PHPCS results in the GitHub Action logs.

When running PHPCS scans (both for checking coding standards and PHP version compatibility), the results are currently only returned silently in a format that GitHub can consume for contextually annotating any code being flagged.

This changes workflows using PHPCS to also display the results of each scan in the GitHub Action log, making it easier to find and understand what is causing failures.

Props jrf.
See #55652.

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

Thanks @desrosj !

Thanks @desrosj !

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

[desrosj]

In 54373:

Build/Test Tools: Update github-script action to the latest version.

The latest version of the actions/github-script action fixes an issue where passing options to the action would remove any default values not passed (see ​https://github.com/actions/github-script/pull/293).

This also includes updates to other third-party actions, bringing all third-party versions in Core workflows to their latest versions:

• actions/cache
• actions/setup-node
• codecov/codecov-action

See #55652.

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

Addressed in https://core.trac.wordpress.org/changeset/54342 and https://core.trac.wordpress.org/changeset/54343.

[TobiasBg]

@jrf and @desrosj:

A note regarding continue-on-error: true in [54371], if issues arise:

I've recently had issues with that continue-on-error (for the exact same use case of getting PHPCS output), as this apparently can lead to the "job" being marked as "passing" when the "step" however failed, and have since instead switched to using if: success() || failure() in the step that calls cs2pr.

[jrf]

Replying to TobiasBg:

@jrf and @desrosj:

A note regarding continue-on-error: true in [54371], if issues arise:

I've recently had issues with that continue-on-error (for the exact same use case of getting PHPCS output), as this apparently can lead to the "job" being marked as "passing" when the "step" however failed, and have since instead switched to using if: success() || failure() in the step that calls cs2pr.

@TobiasBg Could you provide a little more detail about the specific situation in which it caused you problems ?

I mean, I have been using this two-step approach in dozens of projects and I have not seen any issues with it yet, so I'd like to understand if the issue you ran into applies to the WP workflow or not.

Also note: cs2pr will exit with a non-zero exit code if the input file contains errors/warnings/notices. You can turn that off, but that's not being done in these workflows.

[TobiasBg]

@jrf: I can't seem to find the GH Actions run for my plugin where I had this problem, but I have some info from my commit message from the change:

GitHub Actions: Use a different way for executing steps after a previous step errors.
Using continue-on-error achieves the goal, but also lets jobs and workflows pass, which e.g.
leads to the problem that the failed-workflow step is not executed.
The if condition achieves this. Using success() || failure() over always() has the
advantage that it still allows for builds to be cancelled.

(I had been working on copying that failed-workflow enhancement that @desrosj added, as I'm often running into Github API issues, likely related to Composer.)

Now, indeed, if cs2pr will give a non-0 exit code, things should actually be fine.
I'll try again with my change reverted, to see if I can trigger the wrong behavior again.

[SergeyBiryukov]

In 54394:

Tests: Ignore EOL differences in Style Engine API tests.

Unix vs. Windows EOL style mismatches can cause misleading failures in tests using the heredoc syntax (<<<) or multiline strings as the expected result.

This resolves two failures when running the test suite on Windows along the lines of:

1) Tests_Style_Engine_wpStyleEngineCSSRule::test_should_prettify_css_rule_output
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'.baptiste {
-       margin-left: 0;
-       font-family: Detective Sans;
+'.baptiste {
+       margin-left: 0;
+       font-family: Detective Sans;
}'

/var/www/tests/phpunit/tests/style-engine/wpStyleEngineCssRule.php:159

Follow-up to [46612], [48443], [48466], [49691], [51135], [53282], [53319], [54156].

See #56467, #55652.

[SergeyBiryukov]

In 54401:

Tests: Bring some consistency to WP_Image_Editor_GD and WP_Image_Editor_Imagick tests.

Includes:

• Adjusting test method descriptions and comments per the documentation standards.
• Creating image editor class instances directly, instead of calling wp_get_image_editor().
• Cleaning up temporary files before performing assertions, where possible.
• Using more consistent variable names for image editor class instances.
• Reordering some test methods.

Follow-up to [1182/tests], [1188/tests], [27794], [30549], [30990], [31040], [39580], [40123], [49230], [49488], [49542], [49751].

See #55652.

[SergeyBiryukov]

In 54403:

Tests: Clean up test image for site icon in Tests_REST_Server on teardown.

This makes sure there are no leftover images after the test class is run.

Follow-up to [52080], [53463], [53920].

See #55652.

[peterwilsoncc]

In 54407:

Build/Test tools: Add tests for wp_nonce_url().

Props pbearne, costdev.
See #55652.
Fixes #54870.

[TobiasBg]

Replying to TobiasBg:

A note regarding continue-on-error: true in [54371], if issues arise:

I can't seem to reproduce this anymore, and as Core wasn't seeing this problem anyways, that note can be disregarded.

[desrosj]

Is ​this maybe what you were thinking of @TobiasBg?

continue-on-error is supported at the job and step level, but how these are reflected in the various UI locations is pretty inconsistent. Sometimes the entire workflow or job will show as failed, and others it will show as passing, even though a step failed.

[TobiasBg]

Replying to desrosj:

Is ​this maybe what you were thinking of @TobiasBg?

I hadn't seen that discussion yet, thanks for the link! Yes, my issues are in that direction, where jobs were marked as passing, while a step failed. (And I'm battling GitHub API rate limits a lot, even though I'm not really doing anything with them, besides a bit Composer ...)

[SergeyBiryukov]

In 54424:

Tests: Remove unnecessary file copying in WP_Customize_Manager tests.

This was added to avoid creating leftover image sub-sizes in the version-controlled DIR_TESTDATA directory.

However, this does not appear to be necessary:

• WP_Customizer_Manager::import_theme_starter_content() already makes a copy of the image​ before sideloading, so the test was essentially working with a copy of a copy.
• The images were only used in one test out of 70 and do not need to be copied for every single test.

Upon further investigation, there is also no evidence that creating these copies actually resolved the reported issue:

• WP_UnitTest_Factory_For_Attachment::create_object() inserts an attachment, but does not create image sub-sizes.
• media_handle_sideload() does create image sub-sizes, but the file is already in the media library by that time, and sub-sizes are created in the uploads directory, not in the version-controlled DIR_TESTDATA directory.

This commit removes ~140 redundant file copying operations when running the test suite.

Follow-up to [39276], [39346], [39411], [40142].

See #55652.

[SergeyBiryukov]

In 54425:

Tests: Clean up test images in WP_Customize_Manager tests.

The test for WP_Customizer_Manager::import_theme_starter_content() creates two attachments that remain in the uploads directory after the test run is complete.

This commit follows the approach from WP_REST_Posts_Controller tests and utilizes an $attachments_created property to track any files uploaded in the current test run and clean them up afterwards.

This makes sure there are no leftover images after the test class is run.

Follow-up to [39276], [39346], [39411], [40142], [53935], [54424].

See #55652.

[SergeyBiryukov]

In 54426:

Coding Standards: Correct alignment in test_import_theme_starter_content().

This fixes an Equals sign not aligned correctly WPCS warning.

Follow-up to [54425].

See #55652.

[SergeyBiryukov]

In 54428:

Tests: Minimize file copying in WP_REST_Attachments_Controller tests.

The tests use two images that were deleted on teardown and recreated on setup for every single test. This appears to be unnecessary, as the files can instead only be recreated if they are missing, and deleted after the test run is complete.

This commit reduces ~200 redundant file copying operations to ~5 when running this test class.

Follow-up to [38832], [48291], [54424].

See #55652.

[desrosj]

With RC1 later today, going to close this out. #56793 was created for 6.2.

If more required test changes come up during RC, then they can also make reference to this ticket.

Follow up on PR #3368 and in particular @TobiasBg's remark in https://core.trac.wordpress.org/ticket/55652#comment:203

Ha! Found a situation which would allow the build to pass, even though the CS step would fail (but it would do so silently due to the continue-on-error)

If there is a ruleset error, the cs2pr action doesn't receive an xml report and exits with a 0 error code, even though the PHPCS run failed (though not on CS errors, but on a ruleset error).

This changes the GH Actions workflow to allow for that situation and still fail the build in that case.

Trac ticket: https://core.trac.wordpress.org/ticket/55652
Trac ticket: https://core.trac.wordpress.org/ticket/56793

Follow up PR #3516

[desrosj]

In 54674:

Build/Test Tools: Hardcode the ref for the workflow dispatch on failure.

This removes the dynamic aspect of the createWorkflowDispatch() call that dispatches a Failed Workflow run when another workflow encounters an issue.

By hardcoding trunk as the ref, the version of the workflow used will always be the latest, most up to date. This ensures older branches receive the bug fixes and improvements made in trunk without having to backport them.

See #55652.

[desrosj]

Reopening so that [54674] can recieve a second sign off for backporting to the 6.1 branch.

This looks good, though there is one thing I am wondering.

If the "Run PHPCS on all Core files" step fails, the corresponding cs2pr step will still run. But I'm not sure that the "Check test suite files for warnings" and second cs2pr step will also run if there's a preceding failure.

Should "Check test suite files for warnings" be updated to have an if: ${{ always() }} conditional?

If the "Run PHPCS on all Core files" step fails, the corresponding cs2pr step will still run. But I'm not sure that the "Check test suite files for warnings" and second cs2pr step will also run if there's a preceding failure.

They won't.

I get where you are coming from with this question, but that discussion is outside the scope of this PR.

This PR doesn't change that behaviour, the "test cs" check was always skipped if the "src cs" check failed. I didn't set it up like that 🤷🏻‍♀️

Should "Check test suite files for warnings" be updated to have an if: ${{ always() }} conditional?

If you want my advise on how to improve it, I would actually suggest adding a matrix toggle and having one build running the "src cs" check and another the "test cs" check (using the same workflow, with both current steps getting a condition based on the toggle).
You could even add the PHPCompatibility check to the matrix as well then and get rid of the extra (duplicate) workflow.

Happy to help make that change, but, as I said, that's for another PR.

[desrosj]

In 54678:

Build/Test Tools: Ensure PHPCS related workflows are properly marked as failed.

When a ruleset error is encountered during a PHPCodeSniffer scan, an XML report is not generated and cs2pr will exit with a 0.

In this situation, a workflow run will be marked as passing (even though a failure has occurred) due to the presence of continue-on-error.

This adjusts the logic in the Coding Standards and PHP Compatibility workflows to remove the need for the continue-on-error option and ensures all failures are accurately reflected within the GitHub Actions UI.

Follow up to [54371].

Props jrf, TobiasBg.
See #55652.

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

[desrosj]

Both [54674] and [54678] need a second committer sign off to backport.

[azaozz]

Replying to desrosj:

Both [54674] and [54678] need a second committer sign off to backport.

As far as I see these changes are only to build/test functionality, not to "production" files. They will not affect how WP works, so don't think they need a second committer sign-off for backporting.

At the same time I don't see how the changes can be tested (locally). The only way seems to be to leave them in trunk for coupe days and see how they work. Perhaps can be backported after?

[desrosj]

Replying to azaozz:

As far as I see these changes are only to build/test functionality, not to "production" files. They will not affect how WP works, so don't think they need a second committer sign-off for backporting.

Ah, thanks. I honestly couldn't recall if build/test tool, tests, and docs were allowed without a double sign off. So wanted to follow the process to be safe.

At the same time I don't see how the changes can be tested (locally). The only way seems to be to leave them in trunk for coupe days and see how they work. Perhaps can be backported after?

Correct, these are GHA platform specific changes. I'm pretty confident in them, so I'll backport now. If follow up adjustments are needed, they can be included as part of #56882, which will backport a handful of other GHA related changes to older branches to ensure our workflows continue to run without issue going forward.

[desrosj]

In 54680:

Build/Test Tools: Hardcode the ref for the workflow dispatch on failure.

This removes the dynamic aspect of the createWorkflowDispatch() call that dispatches a Failed Workflow run when another workflow encounters an issue.

By hardcoding trunk as the ref, the version of the workflow used will always be the latest, most up to date. This ensures older branches receive the bug fixes and improvements made in trunk without having to backport them.

Merges [54674] to the 6.1 branch.
See #55652.

[desrosj]

In 54681:

Build/Test Tools: Ensure PHPCS related workflows are properly marked as failed.

When a ruleset error is encountered during a PHPCodeSniffer scan, an XML report is not generated and cs2pr will exit with a 0.

In this situation, a workflow run will be marked as passing (even though a failure has occurred) due to the presence of continue-on-error.

This adjusts the logic in the Coding Standards and PHP Compatibility workflows to remove the need for the continue-on-error option and ensures all failures are accurately reflected within the GitHub Actions UI.

Follow up to [54371].

Props jrf, TobiasBg.
Merges [54678] to the 6.1 branch.
See #55652.

[azaozz]

Replying to desrosj:

I honestly couldn't recall if build/test tool, tests, and docs were allowed without a double sign off.

Yep, that part of the docs/handbook needs to be updated to make this clearer. As far as I know changes to tests, build/test tools, and inline comments (docblocks, etc.) do not require double sign-off. The reason is that these don't affect (directly) how the new release works in production.

A special case that does not require double sign-off are changes to the translatable strings. It is quite undesirable to change them as strings should be in string-freeze, but it's still possible. Usually these changes are mostly to fix a typo, etc.

[desrosj]

In 55516:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.

Merges [53736], [53737], [53940], [53947], [54039], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54649], [54650], [54651], [54674], [54750], [54852], [55152], [55487] to the 5.9 branch.
See #55652, #56407, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55517:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54649], [54650], [54651], [54674], [54750], [54852], [55152], [55487] to the 5.8 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55518:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54649], [54650], [54651], [54674], [54750], [54852], [55152], [55487] to the 5.8 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55519:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54649], [54650], [54651], [54674], [54750], [54852], [55152], [55487] to the 5.6 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55520:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54649], [54650], [54651], [54674], [54750], [54852], [55152], [55487] to the 5.5 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55521:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54649], [54650], [54651], [54674], [54750], [54852], [55152], [55487] to the 5.4 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55522:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54649], [54650], [54651], [54674], [54750], [54852], [55152], [55487] to the 5.3 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55523:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54649], [54650], [54651], [54674], [54750], [54852], [55152], [55487] to the 5.2 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55524:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 5.1 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55525:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 5.0 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55527:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 4.9 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55528:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 4.8 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55529:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 4.7 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55530:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 4.6 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55531:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 4.5 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55532:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 4.4 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55533:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 4.3 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55534:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 4.2 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.

[desrosj]

In 55535:

Build/Test Tools: Backport updates to GitHub Actions.

This backports several changesets to GitHub Actions workflows. These changesets:

• address the deprecated notices related to save-output and set-output to ensure the workflows continue to run after these are removed.
• adds support for automatically retrying a failed workflow once.
• removes workflow files that are not applicable to the branch.
• backports some Docker environment related tooling updates for the sake of consistency across branches.

Merges [53736], [53737], [53940], [53947], [54039], [54096], [54108], [54293], [54313], [54342], [54343], [54373], [54511], [54650], [54651], [54674], [54750], [54852], [55152], [54651], [55487] to the 4.1 branch.
See #55652, #56407, #56528, #54695, #56820, #56816, #56793, #56820, #57572.