Working on #3031379: Add a new test type to do real update testing we end up running tests from PHPUnit a lot.
PHPUnit shows us an error like this:
$ ./vendor/bin/phpunit -c core --testsuite unit
[...]
........................................................... 2655 / 17890 ( 14%)
........................................................... 2714 / 17890 ( 15%)
........................................................... 2773 / 17890 ( 15%)
........................................................... 2832 / 17890 ( 15%)
......................................
THE ERROR HANDLER HAS CHANGED!
One of the unfortunate things about symfony/phpunit-bridge is that it doesn't tell us very much about what happened here. Watching the PHP error log, we see:
PHP Fatal error: Cannot redeclare drupal_valid_test_ua() (previously declared in /Users/paul/projects/drupal/core/tests/Drupal/Tests/Core/DrupalKernel/DrupalKernelTest.php:249) in /Users/paul/projects/drupal/core/includes/bootstrap.inc on line 648
Sure enough, fixing this error so that we don't redeclare drupal_valid_test_ua() in the test makes the error handler error go away.
The cause here is likely similar to #3052625: DeprecationListenerTrait croaks on tests with no results in that the test that has a fatal does not have any results, so the listener is unable to register other error handlers properly.
Don't redeclare drupal_valid_test_ua() in DrupalKernelTest.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3054649-12.patch | 455 bytes | alexpott |
| #9 | 3054649_8.patch | 1.67 KB | mile23 |
Comments
Comment #2
mile23A patch.
Comment #3
mile23Comment #4
mile23Results: Test Result (1,488 failures / +1488)
Unrelated? I hope?
Comment #6
berdirDoes #3038513: Move drupal_generate_test_ua() into the test system fix this too? I guess we can't backport that, though.
Comment #7
mile23If #3038513: Move drupal_generate_test_ua() into the test system is really scoped for D9 then it won't fix it before then.
Comment #8
berdirThat's just a regular deprecation issue, the only D9 thing is that the old methods will be removed then.
Comment #9
mile23#2 failed because
testInitializeSettings()says@requires function apcu_fetch, which ended up as skipped on my machine. Therefore I didn't notice it.Comment #10
jibranThis makes sense to me.
Comment #11
alexpottHmmm... so the question here is what is defining or including bootstrap.inc elsewhere. Imo we should not be doing an include here without running all the tests in a separate process.
Comment #12
alexpottThis fixes things in a way that \Drupal\Tests\Core\ClassLoader\ClassLoaderTest doesn't affect the loaded code for all unit tests that come after it.
Comment #13
berdirThat seems like a good idea, confirmed that it fixes the error. Additionally, this is a legacy test, so it will go away in D9.
Comment #14
catchCommitted/pushed to 8.8.x and 8.7.x, thanks!
Comment #17
mile23It looks like 8.7.x didn't have this test before it was pushed, resulting in a branch fail: https://www.drupal.org/pift-ci-job/1334211