Opened 4 years ago
Last modified 4 years ago
#52610 new task (blessed)
Consider removing many of the default test group exclusions
Reported by: | johnbillion | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | needs-patch dev-feedback |
Focuses: | multisite | Cc: |
Description (last modified by )
When the tests are run with composer test
, the following groups are excluded:
ajax
ms-files
ms-required
external-http
When the tests are run with Multisite enabled with composer test -- -c tests/phpunit/multisite.xml
, the following groups are excluded:
ajax
ms-files
ms-excluded
external-http
oembed-headers
The ms-required
and ms-excluded
group exclusions are needed so that the Multisite-specific tests and single-site-specific tests don't run when they don't need to.
It's less clear why the other groups in these lists are excluded by default.
The ajax
and ms-files
groups are not slow, so excluding them for performance reasons doesn't make sense. I think the ajax
exclusion should be removed from both the single site and Multisite configuration. The ms-files
exclusion should be removed too because the tests in the ms-files
group don't get registered on a non-Multisite test run so the exclusion is redundant.
The external-http
tests are excluded because they are somewhat slow, taking around 10-15 seconds on GitHub Actions and around 40 seconds on my local, highly dependent on network connection speed. Let's keep these excluded by default.
The oembed-headers
group is excluded by default because it requires Xdebug, however this is already covered by the @requires function xdebug_get_headers
tag on the only test in this group, along with being in the debug
group which runs separately on GitHub Actions. The oembed-headers
group exclusion can be removed as it's redundant.
Here's my proposed new config for phpunit.xml.dist
:
<exclude> <group>ms-required</group> <group>external-http</group> </exclude>
and for multisite.xml
:
<exclude> <group>ms-excluded</group> <group>external-http</group> </exclude>
Change History (6)
#3
@
4 years ago
Some history here:
- [924/tests] / #tests49 #tests42 excluded the
ajax
group for performance reasons. - [30298] / #30304 excluded the
external-http
group for performance reasons. - [30286] / #30256 excluded the
ms-files
group from multisite config for a more consistent environment. - [37269] / #36566 excluded the
ms-files
group from single site config to avoid pollution when mixed with the rest of the tests. - [40520] / #40531 introduced the
ms-required
andms-excluded
groups to reduce the number of skipped tests polluting PHPUnit's output.
#4
@
4 years ago
Ah yes, the ms-files
tests needs to remain separate because the constants in ms_upload_constants()
are set based on the value of the ms_files_rewriting
option, which is itself set in the @group ms-files
tests. Removing ms-files
from the excluded groups causes many failures.
#5
@
4 years ago
The ajax
group exclusion hasn't been needed since [41293]. It was introduced in [924/tests] because of the reliance on DOING_AJAX
which is no longer the case.
#6
@
4 years ago
Ah yes, the ms-files tests needs to remain separate because the constants in ms_upload_constants() are set based on the value of the ms_files_rewriting option, which is itself set in the @group ms-files tests. Removing ms-files from the excluded groups causes many failures.
Just a heads-up: you can declare multiple testsuite
entries in <testsuites>. If the
ms-files` group gets set as a separate testsuite, it will probably not need a separate run and prevent the errors you are seeing. At least worth giving it a try. I remember fixing a similar situation not too long ago this way.
Removing the exclusion for
ms-files
from the single site config makes sense.Via [30286]:
I think this remains a reason to continue excluding
ms-files
in the multisite test config.