Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Here's a case where you discover test fails under PHPUnit that you do not see under run-tests.sh: #2905470: @deprecated Drupal\system\Tests\Update\UpdatePathTestBase breaks test runs
Why does this happen?
run-tests.sh runs all PHPUnit tests in a separate process. This is good for a number of reasons. However, because run-tests.sh has already discovered the tests, the phpunit child process doesn't need to. run-tests.sh just passes along the file path on the command line.
This is good because it's fast and good because there's no ambiguity.
But it's bad because our deprecation process includes putting @trigger_error() in non-executable portions of files. That is, when you put @trigger_error(E_USER_DEPRECATED) just after the namespace declaration, that error will only be triggered during discovery.
So we have a situation in #2905470: @deprecated Drupal\system\Tests\Update\UpdatePathTestBase breaks test runs where all PHPUnit-based functional tests (at least) should fail, by design, because of our deprecation process. But they do not fail, because PHPUnit never performs discovery.
This means run-tests.sh is giving us false passes for all PHPUnit functional tests.
Proposed resolution
Instead of launching each PHPUnit child process with a file path to the PHPUnit test class, we should force the child process to do discovery using --filter.
We can minimize the time this takes by calculating the test suite of the test class in question and then providing --testsuite on the command ilne as well.
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 2906009_5_2905470_4_combined.patch | 3.11 KB | Mile23 |
| #2 | 2906009_2.patch | 1.77 KB | Mile23 |
Comments
Comment #2
Mile23Here's a patch.
This test will fail, in a big way, which means it's working.
Comment #3
Mile23Comment #5
Mile23And here we have the patch from #2 combined with the patch from #2905470-4: @deprecated Drupal\system\Tests\Update\UpdatePathTestBase breaks test runs
Note that #2905470: @deprecated Drupal\system\Tests\Update\UpdatePathTestBase breaks test runs then solves the bug we surfaced here.
Comment #6
dawehnerCan you explain why?
I am wondering whether there is a way to fix that. Running everything a different way than people might use it locally feels like a hacky fix.
Comment #8
Mile23You're welcome.
Yes.
#2870194-31: Ensure that process-isolated tests can use Symfony's PHPunit bridge to catch usages of deprecated code
#2884530: Mark process-isolated test with @expectedDeprecation as risky
#2886547-22: Add trigger_error to deprecated TestBase base classes/traits that have none, reference CR
Those all explain that
BrowserTestBaseand other process-isolated tests will ignore @trigger_error() in all its forms. It's an upstream problem with symfony/phpunit-bridge, so if I solve it here, no one will a) understand it or b) want it, as evidenced by the above comment about hacky fixes.This issue cuts some of the complexity off the problem for one case: @trigger_error() before executable code in a class file. We can at least catch those by forcing phpunit to do discovery instead of having all @trigger_error() being swallowed by a child process.
And the reason we want this is because run-tests.sh has false passes, when it should have failed at least two tests, and probably all functional tests, as seen in #2905470: @deprecated Drupal\system\Tests\Update\UpdatePathTestBase breaks test runs
If the policy here is that test should fail because they use deprecated base classes, then this is a no-brainer.
If that's not the policy then I am, truly, wasting my time on hacky fixes.
Comment #9
Mile23I made a 'blog post about this. http://mile23.com/content/how-phpunit-and-phpunit-bridge-make-false-pass...
Comment #14
quietone CreditAttribution: quietone as a volunteer commentedTriaging issues in simpletest.module as part of the Bug Smash Initiative to determine if they should be in the Simpletest Project or core.
run-tests is still in core so moving to the Phpunit component, the only testing related component.