Make WordPress Core

Opened 6 years ago

Last modified 5 months ago

#42093 assigned task (blessed)

Improve handling of SUBDOMAIN_INSTALL test coverage

Reported by: jeremyfelt's profile jeremyfelt Owned by: jeremyfelt's profile jeremyfelt
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: needs-unit-tests has-patch
Focuses: multisite Cc:

Description

We have a bunch of tests that run (or are skipped) based on is_subdomain_install(), but our Travis CI configuration never runs the tests in a subdomain configuration.

To run the tests now, you need to setup the environment manually. In its current state, 7 tests fail:

1) Tests_Multisite_Site::test_created_site_details
2) Tests_Multisite_Site::test_new_blog_url_schemes with data set #0 ('https', 'https', false)
3) Tests_Multisite_Site::test_new_blog_url_schemes with data set #1 ('http', 'https', false)
4) Tests_Multisite_Site::test_new_blog_url_schemes with data set #2 ('https', 'http', false)
5) Tests_WP_oEmbed::test_wp_filter_pre_oembed_result_multisite_sub_samesub
6) Tests_WP_oEmbed::test_wp_filter_pre_oembed_result_multisite_sub_othersub
7) Tests_WP_oEmbed::test_wp_filter_pre_oembed_result_multisite_preserves_switched_state

The first is a result of our site factory only creating new subdirectory sites. I believe the test_new_blog_url_schemes should be skipped for is_subdomain_install(), but I haven't verified. I'm not sure yet what the issue is with the oEmbed tests.

I'd like to:

  • Clean up the existing tests so that they pass.
  • Determine a good way of running these in our Travis CI configuration.

It'd be nice *not* to run the entire set of core/multisite tests again just to account for this. Hopefully there's a creative approach to run only the subdomain specific tests.

Previously: #36567, #36566.

Change History (11)

#1 @johnbillion
6 years ago

  • Keywords needs-patch needs-unit-tests added

Brain dump: Allow is_subdomain_install() return value to be filtered, remove usage of SUBDOMAIN_INSTALL constant from tests, allow both subdomain and subdirectory tests to run during one test suite run.

#2 follow-up: @johnbillion
6 years ago

Alternatively, it would be neat if all of the Multisite tests ran once under a subdirectory installation and once under a subdomain installation, to ensure everything works on both.

#3 @jeremyfelt
6 years ago

  • Owner set to jeremyfelt
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


6 years ago

#5 in reply to: ↑ 2 @jeremyfelt
6 years ago

  • Milestone changed from 4.9 to 5.0

Replying to johnbillion:

Alternatively, it would be neat if all of the Multisite tests ran once under a subdirectory installation and once under a subdomain installation, to ensure everything works on both.

This is probably the right answer, though it adds a good amount of time to the total CI run. I guess it's our fault for having all of these possible configurations. :)

I'm going to move this to the 5.0 milestone for now. If we start making some progress, then we can still shift to 4.9 for test changes.

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


6 years ago

#7 @jorbin
5 years ago

  • Milestone changed from 5.0 to 5.1

#8 @pento
5 years ago

  • Milestone changed from 5.1 to Future Release

This ticket was mentioned in PR #5522 on WordPress/wordpress-develop by @jeremyfelt.


5 months ago
#9

  • Keywords has-patch added; needs-patch removed

@jeremyfelt commented on PR #5522:


5 months ago
#10

@desrosj Super low priority, I'm just poking to see if I can get this to work. 😄

Is there a best practice for modifying php-tests-run.yml without changing this line?

https://github.com/WordPress/wordpress-develop/blob/545748c9d26cad8d1565d1acf90cbd75e9d4df84/.github/workflows/phpunit-tests.yml#L39

Or can I tweak that and then revert to test this PR?

@desrosj commented on PR #5522:


5 months ago
#11

Or can I tweak that and then revert to test this PR?

Unfortunately, no. I usually just change it to point to the version in my fork in the PR's branch. jeremyfelt/wordpress-develop.....@add-subdomain-tests in this case.

Just have to remember to change it back when committing to trunk. Adding a comment to that line in the PR could help serve as a call out for that.

Note: See TracTickets for help on using tickets.