Test tool and unit test improvements for 6.7

[jonsurrell]

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

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

Add tests for comment handling.

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

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

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

Trac ticket: Core-61530

Add tests for comment handling.

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

[SergeyBiryukov]

In 58594:

Tests: Use assertSame() in WP_Interactivity_API tests.

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

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

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

See #61530.

Trac ticket: Core-61530

Add tests for comment handling.

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

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

Trac ticket: Core-61530

Add tests for comment handling.

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

Trac ticket: Core-61530

Add tests for comment handling.

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

Trac ticket: Core-61530

Add tests for comment handling.

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

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

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

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

[jrf]

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

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

[hellofromTonya]

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

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

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

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

Runtime:       PHP 8.2.21

After this patch, the deprecation is no longer thrown ✅

Marking for and preparing the commit.

[hellofromTonya]

In 58733:

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

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

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

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

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

Follow-up to [58712].

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

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

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

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

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

Using a closure for this callback fixes the issue.

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

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

This is better as:

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

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

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

This is better as:

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

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

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

This is better as:

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

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

[jrf]

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

[dmsnell]

In 58734:

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

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

This patch adds the missing tests.

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

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

Props dmsnell, jonsurrell.
See #61530.

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

Trac ticket: Core-61530

Add tests for comment handling.

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

[hellofromTonya]

Replying to jrf:

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

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

[hellofromTonya]

In 58738:

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

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

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

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

Follow-up to [57826].

Props jrf, hellofromTonya.
See #61530.

[hellofromTonya]

In 58739:

Tests: Use data provider in Tests_Interactivity_API_wpInteractivityAPIFunctions.

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

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

This is better as:

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

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

Props jrf.
See #61530.

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

Thanks @hellofromtonya!

[SergeyBiryukov]

In 58803:

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

Follow-up to [58335].

See #61530.

[hellofromTonya]

In 58919:

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

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

Why?

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

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

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

See #61890, #61530.

[hellofromTonya]

In 58920:

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

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

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

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

See #61890, #61530.

[SergeyBiryukov]

In 58954:

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

Follow-up to [56817].

See #61530.

[SergeyBiryukov]

In 58968:

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

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

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

[peterwilsoncc]

In 59015:

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

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

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

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

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

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

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

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

[jrf]

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

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

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

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

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

[jrf]

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

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

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

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

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

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

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

[jrf]

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

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

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

[jrf]

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

[hellofromTonya]

In 59054:

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

Uses DIRECTORY_SEPARATOR in closures for cross-OS differences.

Follow-up to [56635].

Props jrf.
See #61530.

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

[hellofromTonya]

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

In 59057:

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

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

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

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

Props jrf, hellofromTonya.
See #61530.

[hellofromTonya]

In 59061:

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

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

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

Props jrf.
See #61530.

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

[SergeyBiryukov]

In 59082:

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

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

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

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

Reference: ​Composer Documentation: Config: lock.

Follow-up to [51543].

Props jrf.
See #61530.

Thanks for the PR! Merged in r59082.

Thanks @SergeyBiryukov!

[SergeyBiryukov]

In 59085:

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

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

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

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

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

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

Props jrf, hellofromTonya.
See #61530.

Thanks for the PR! Merged in r59085.

[bjorsch]

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

[jrf]

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

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

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

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

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

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

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

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

[ramonopoly]

Are there any updates required in the Gutenberg repo?

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

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

[jrf]

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

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

[jrf]

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

[ramonopoly]

Thanks for the quick reply and context @jrf

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

Thank you!

[bjorsch]

Replying to jrf:

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

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

[jrf]

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

[bjorsch]

Replying to jrf:

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

Yes, it did. 🙂

[peterwilsoncc]

In 59116:

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

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

See #61530.

Additional tests for default themes:

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

Trac ticket: Core-61530

[SergeyBiryukov]

In 59163:

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

Includes:

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

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

See #61530.

[peterwilsoncc]

In 59186:

Tests/Build Tools: Improve tests for bundled themes.

Introduce two new tests relating to bundled themes:

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

See #61530.

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

Trac ticket: Core-61530

[desrosj]

In 59212:

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

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

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

See #61530.

This allows old versions to continue running E2E tests that require the Gutenberg plugin even after the minimum required version of Wordpress is increased for the plugin.

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

[desrosj]

In 59220:

Build/Test Tools: Add input for Gutenberg version to E2E workflow.

The E2E test suite has some basic tests to confirm that the plugin can be activated without conflicts. This works great until the minimum version of WordPress required to run the plugin is raised. Old branches that attempt to run these tests that are newly incompatible with the latest version of the plugin will fail.

This change adds a gutenberg-version input to the E2E GitHub Action workflow to allow these tests to continue running with the last compatible version.

Props swissspidy.
See #61530.

Fixed in https://core.trac.wordpress.org/changeset/59220.

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

[desrosj]

In 59221:

Build/Test Tools: Use a specific version of Gutenberg for E2E tests.

This updates the 6.4 branch to make use of the new gutenberg-version input introduced in [59220].

The past version of the plugin that supports WordPress 6.4 is 18.9.0.

Props swissspidy.
See #61530.

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

[peterwilsoncc]

In 59227:

Bundled Themes: Test new themes are added to new bundled files.

Introduces a test to ensure that new bundled themes are included in the $_new_bundled_files array. Modifies previous theme related tests added during the 6.7 release cycle to include the correct annotations.

Props peterwilsoncc, mukesh27, costdev.
See #61530, #62103.

Merged r59227 / ​https://github.com/WordPress/wordpress-develop/commit/1dcbe1096e14347c0c9913ec9fb985622d9b1285

Anything for the actions can be added in a follow up.

[SergeyBiryukov]

In 59243:

Tests: Correct test class name for rss_enclosure() as per the naming conventions.

Includes:

• Moving @covers tags to the class-level DocBlock, since this is the only function tested in the class.
• Adjusting the assertion messages for clarity, as rss_enclosure() does not return anything directly.
• Using a static closure for improved performance.

Follow-up to [59241].

See #61530.

[Rahmohn]

Hello,

Could someone review this PR ​https://github.com/WordPress/wordpress-develop/pull/4132?

I originally created it for ticket #56800 ("Reduce usage of assertEquals"), but I think it's more of an improvement.

Thanks.

[desrosj]

Hey @Rahmohn,

My apologies that we were unable to get that one over the finish line. RC1 is today for 6.7 so I'm going to close this one out. I've opened #62278 for the 6.8 cycle and updated your PR to reference that one instead.

That said, test updates can happen at any time, so if a committer has time to review before 6.7 is released, it can make it's way in attached to this ticket.

I've opened #62280 for test tool and unit test improvements in 6.8.

[azaozz]

Just ran into the error: "The test suite requires the WordPress Importer plugin to be available in the /data/plugins/ directory." on a fresh git clone running wp-env.

Sorry but [59085] doesn't seem to make sense. Why is a WordPress plugin required to be present at an arbitrary location, not in the wp-content/plugins directory, to be able to run unit tests? This seems contraintuitive, the PHP unit tests can be run only partially, or while developing a plugin, etc. It doesn't seem to make sense to prevent running any tests just because some non-core file is not present. Seems a better solution would be to skip any core tests that require external/extra files when they are not present?

Also if any external files are required when testing core, including any WP plugins, they should either be committed to the appropriate locations or added during building, just like all the image test files, etc. It doesn't make sense to require files to be added by hand. That seems pretty bad UX :)

Reopening as this ticket is a task and (maybe) can be fixed during RC too.

[peterwilsoncc]

@azaozz The test suite requiring the importer be in a defined location was always the case, if that changes it can be done so during the 6.8 cycle.

The change in [59085] changed how the tests failed, stalling the entire test suite rather than failing the individual tests.

For 6.7, let's consider whether we should revert back to failing only the individual tests rather than existing early. PR incoming for people to consider.

Follow up to r59085.

Instead of preventing the test suite from running, only fail the import related tests if the plugin is not available.

Note, this puts a call to fail in the setIUp of several tests. It may need to be moved to each individual test if there is a risk that PHP Unit will stop accepting fail calls during the set up.

Trac ticket: Core-61530

[desrosj]

Related: #42668.

This focuses on removing the importer tests and only running them within the plugin's repository on GitHub instead, which seems like a better solution.

[peterwilsoncc]

@desrosj I agree but for 6.7 I think it would be handy to revert the change so the tests don't exit if the plugin is unavailable.

[desrosj]

Replying to peterwilsoncc:

@desrosj I agree but for 6.7 I think it would be handy to revert the change so the tests don't exit if the plugin is unavailable.

Sorry, that was not clear form my post. Was not advocating for tackling #42668 now and in favor of a revert or smaller change in the mean time for 6.7.

[azaozz]

Replying to peterwilsoncc:

The test suite requiring the importer be in a defined location was always the case
...
The change in [59085] changed how the tests failed, stalling the entire test suite rather than failing the individual tests.

Right, it's not a huge problem just unexpected/not a good UX when working with the git mirror/GitHub.

For 6.7, let's consider whether we should revert back to failing only the individual tests rather than existing early.

Sounds good!

[azaozz]

Replying to desrosj:

Related: #42668.

This focuses on removing the importer tests and only running them within the plugin's repository

Yep, that would be best. Changed the milestone there to 6.8.

Thinking perhaps the same approach can be used in other cases. For example the PHP tests may run very slow, and some would fail if the internet connection is slow or flaky. Currently there are nearly 25,000 tests. With a slow/flaky connection they may take 15-20 minutes per run (using Docker/wp-env). Seems that can be improved by skipping tests that require internet to run when they are not essential?

Seems that can be improved by skipping tests that require internet to run when they are not essential?

There must be some tests missing the http group that ought to have it as network tests ought to run only when the group is specified. I'll open a ticket.

Sorry, the group name is external-http, as noted on the ticket I've created.

Merged r59326 / ​https://github.com/WordPress/wordpress-develop/commit/023af2111b9bbf2fa00705227b9f90ea537cc889

[peterwilsoncc]

Now that the importer test changes have been improved (if I may say so myself), I'll close this off again. Thanks for raising this Ozz!

[peterwilsoncc]

I used the wromng ticket number in the commits:

In r59326:

Tests/Build tools: Only fail importer tests if plugin is missing.

Reverts an earlier change to the test suite in which the PHPUnit tests could not run if the importer plugin was not available.

This update allows the test suite to run and will fail importer tests if the plugin is not available.

Follow up to r59085.

Props peterwilsoncc, azaozz.

Backported to 6.7 in r59327.