WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 3 months ago

#24173 new defect (bug)

Unit tests: Support subdirectory multisite installs

Reported by: r-a-y Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: has-patch
Focuses: multisite Cc:

Description

I have unit tests set up at:
localhost/wordpress-tests/

And have set:
define( 'WP_TESTS_MULTISITE', true );

However, currently, unit testing doesn't support subdirectory multisite installs. It fails when installing multisite.

Attached patch addresses this and introduces a new constant called 'WP_TESTS_PATH'.

If this is set in wp-tests-config.php, this will make sure that PHPUnit can install WP for subdirectory multisite installs.

Attachments (2)

24173.01.patch (7.8 KB) - added by r-a-y 12 months ago.
Updated patch to fix WP_SITEURL
24173.02.patch (7.5 KB) - added by r-a-y 10 months ago.

Download all attachments as: .zip

Change History (16)

comment:1 r-a-y12 months ago

  • Keywords has-patch removed

Patch also fixes a few of the multisite unit tests.

Multisite tests still fail in three cases:

There were 3 failures:

1) Tests_MS::test_create_and_delete_blog
Failed asserting that false matches expected 989.

wordpress-tests\tests\ms.php:34

2) Tests_MS::test_get_blog_details_blog_does_not_exist
Failed asserting that -1 is false.

wordpress-tests\tests\ms.php:560

3) Tests_MS::test_update_blog_status
Failed asserting that 16 matches expected 17.

wordpress-tests\tests\ms.php:728

I don't think these cases are related to the subdirectory change.

Last edited 12 months ago by r-a-y (previous) (diff)

comment:2 r-a-y12 months ago

  • Keywords has-patch added

r-a-y12 months ago

Updated patch to fix WP_SITEURL

comment:3 scribu10 months ago

Related: #19796

comment:4 scribu10 months ago

I don't understand why you need a new WP_TESTS_PATH constant. Just define ABSPATH directly in wp-tests-config.php.

comment:5 scribu10 months ago

  • Cc scribu+wptests@… added

comment:6 markoheijnen10 months ago

What is the difference between PATH_CURRENT_SITE and WP_TESTS_PATH?

comment:7 scribu10 months ago

Yeah, even if defining ABSPATH is not enough, you could define PATH_CURRENT_SITE in wp-tests-config.php and then check if it's already defined when handling WP_TESTS_MULTISITE.

comment:8 r-a-y10 months ago

Just define ABSPATH directly in wp-tests-config.php.

ABSPATH is already defined in wp-tests-config.php and that doesn't need to change because it is referencing the correct directory.

What is the difference between PATH_CURRENT_SITE and WP_TESTS_PATH?

PATH_CURRENT_SITE is defined in wpmu_current_site() in ms-load.php. I don't think there's a difference between the two.

you could define PATH_CURRENT_SITE in wp-tests-config.php and then check if it's already defined when handling WP_TESTS_MULTISITE.

I created WP_TESTS_PATH because it is similar to how WP_TESTS_DOMAIN is used.

Similarly, I could ask why WP_TESTS_DOMAIN is used when we can already define DOMAIN_CURRENT_SITE.

comment:9 scribu10 months ago

Similarly, I could ask why WP_TESTS_DOMAIN is used when we can already define DOMAIN_CURRENT_SITE.

That's a good question and the answer is that WP_TESTS_DOMAIN is used on single-site test runs too:

$_SERVER['HTTP_HOST'] = WP_TESTS_DOMAIN;

r-a-y10 months ago

comment:10 r-a-y10 months ago

02.patch replaces WP_TESTS_PATH with PATH_CURRENT_SITE.

Patch is still necessary due to $GLOBALS['base'], $_SERVER['REQUEST_URI'] and WP_SITEURL.

I've tested the patch against the multisite unit tests and it passes all tests.

Last edited 10 months ago by r-a-y (previous) (diff)

comment:11 scribu10 months ago

I'm concerned about this part:

 	60	        // This will override wp_guess_url() when populating the install 
 	61	        // Note: we're suffixing with 'wordpress' since our WP install is in a subdirectory 
 	62	        define( 'WP_SITEURL', 'http://' . WP_TESTS_DOMAIN . $path . 'wordpress' );

It ignores the ABSPATH constant. You might have WP in a subdirectory, but I do not.

comment:12 r-a-y10 months ago

Good point.

The default ABSPATH location in wp-tests-config-sample.php uses /wordpress/ as a subdirectory.

I can probably remove the WP_SITEURL constant and define it myself in my own wp-tests-config.php. What do you think, scribu? Anything else that jumps out at you?

comment:13 scribu10 months ago

I can probably remove the WP_SITEURL constant and define it myself in my own wp-tests-config.php.

Yeah, that's probably for the best. Note that you have wordpress/ hardcoded in a bunch more places in the patch. For example:

$this->assertEquals( 'http://' . $site->domain . $site->path . 'wordpress/wp-content/uploads/' . gmstrftime('

Otherwise, it looks good.

Last edited 10 months ago by scribu (previous) (diff)

comment:14 nacin3 months ago

  • Component changed from Unit Tests to Networks and Sites
  • Focuses multisite added
Note: See TracTickets for help on using tickets.