WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 21 months ago

#45131 closed defect (bug) (fixed)

Paths are incorrect in multisite.xml

Reported by: danielbachhuber Owned by:
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: fixed-5.0
Focuses: multisite Cc:

Description (last modified by danielbachhuber)

In [25294], the base directory for <exclude> files was changed from tests/ to tests/phpunit/tests/, e.g.

<exclude>tests/phpunit/tests/actions/closures.php</exclude>

However, when Grunt runs phpunit -c tests/phpunit/multisite.xml --verbose from the project root, <exclude> paths are calculated relative to the location of multisite.xml.

I think [25294] is wrong, and the path should be:

<exclude>tests/actions/closures.php</exclude>

Maybe PHPUnit changed how it calculate paths? Here's how I discovered it:

From https://github.com/danielbachhuber/wordpress-develop/pull/2

Change History (10)

This ticket was mentioned in Slack in #core-restapi by danielbachhuber. View the logs.


2 years ago

#2 @danielbachhuber
2 years ago

  • Description modified (diff)

#3 @danielbachhuber
2 years ago

In 43769:

REST API: Skip Autosaves controller test for multisite.

There's some PHP 5.2 (cough, cough) edge case where paths calculated differently, possibly caused by differing version of PHPUnit.

See #45132, #45131, #43316.

#4 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 5.0

#5 in reply to: ↑ description @SergeyBiryukov
2 years ago

Replying to danielbachhuber:

Maybe PHPUnit changed how it calculate paths?

It did: https://github.com/sebastianbergmann/phpunit/commit/9740a586a143be686aeab316168dfc02c5fb4a9c

[BC BREAK]

All relative paths in a configuration file are now resolved relative to that configuration file. When upgrading, you may need to update relative paths for the following configurations:

  • testSuiteLoaderFile
  • printerFile
  • testsuites/file
  • testsuites/exclude

Basically, PHPUnit 3.6.x expects a path relative to the checkout root, PHPUnit 4.x.x and later versions expect a path relative to the configuration file.

So [25294] was not wrong, it was just specific to PHPUnit 3.6.x. Before [43768], all excludes were only relevant for PHP 5.2.x / PHPUnit 3.6.x, and later versions just ignored them, so the path didn't matter.

For rest-autosaves-controller.php, I'd suggest including both paths, that should work for all PHPUnit versions.

#6 @SergeyBiryukov
2 years ago

In 43774:

REST API: Restore Autosaves controller test for multisite.

PHPUnit 3.6.x requires exclude and file paths to be relative to the checkout root.

PHPUnit 4.0.0+ requires the paths to be relative to the configuration file.

See #45131.

#7 @SergeyBiryukov
2 years ago

  • Keywords fixed-5.0 added; dev-feedback removed

#8 @desrosj
22 months ago

In 44126:

REST API: Introduce Autosaves controller and endpoint.

  • Adds WP_REST_Autosaves_Controller which extends WP_REST_Revisions_Controller.
  • Autosaves endpoint is registered for all post types except attachment because even post types without revisions enabled are expected to autosave.
  • Because setting the DOING_AUTOSAVE constant pollutes the test suite, autosaves tests are run last. We may want to improve upon this later.

Also, use a truly impossibly high number in User Controller tests. The number 100, (or 7777 in trunk), could be valid in certain test run configurations. The REST_TESTS_IMPOSSIBLY_HIGH_NUMBER constant is impossibly high for this very reason.

Finally, Skip Autosaves controller test for multisite. There's a PHP 5.2 edge case where paths calculated differently, possibly caused by differing version of PHPUnit.

Props adamsilverstein, aduth, azaozz, danielbachhuber, rmccue, danielbachhuber.

Merges [43767], [43768], [43769] to trunk.

See #45132, #45131.
Fixes #45128, #43316.

#9 @jeremyfelt
22 months ago

In 44128:

REST API: Restore Autosaves controller test for multisite.

PHPUnit 3.6.x requires exclude and file paths to be relative to the checkout root.

PHPUnit 4.0.0+ requires the paths to be relative to the configuration file.

Merges [43774] from the 5.0 branch to trunk.

See #45131.

#10 @SergeyBiryukov
21 months ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.