Make WordPress Core

Opened 11 years ago

Last modified 5 years ago

#24173 new defect (bug)

Unit tests: Support subdirectory multisite installs

Reported by: r-a-y's profile r-a-y Owned by:
Milestone: 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 11 years ago.
Updated patch to fix WP_SITEURL
24173.02.patch (7.5 KB) - added by r-a-y 11 years ago.

Download all attachments as: .zip

Change History (18)

#1 @r-a-y
11 years 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 11 years ago by r-a-y (previous) (diff)

#2 @r-a-y
11 years ago

  • Keywords has-patch added

@r-a-y
11 years ago

Updated patch to fix WP_SITEURL

#3 @scribu
11 years ago

Related: #19796

#4 @scribu
11 years ago

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

#5 @scribu
11 years ago

  • Cc scribu+wptests@… added

#6 @markoheijnen
11 years ago

What is the difference between PATH_CURRENT_SITE and WP_TESTS_PATH?

#7 @scribu
11 years 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.

#8 @r-a-y
11 years 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.

#9 @scribu
11 years 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-y
11 years ago

#10 @r-a-y
11 years 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 11 years ago by r-a-y (previous) (diff)

#11 @scribu
11 years 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.

#12 @r-a-y
11 years 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?

#13 @scribu
11 years 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 11 years ago by scribu (previous) (diff)

#14 @nacin
11 years ago

  • Component changed from Unit Tests to Networks and Sites
  • Focuses multisite added

#15 @jeremyfelt
10 years ago

  • Milestone changed from Awaiting Review to Future Release

It is possible that this becomes possible (without the extra constant) along with #27884

#16 @chriscct7
9 years ago

As an update #27884 has been merged

Note: See TracTickets for help on using tickets.