Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
1) Drupal\Tests\Core\Update\UpdateRegistryTest::testGetPendingUpdateFunctionsNoExistingUpdates
PHPUnit_Framework_Exception: PHP Notice: Constant REQUIREMENT_INFO already defined in /.../d8/core/includes/install.inc on line 19
PHP Stack trace:
PHP 1. {main}() -:0
PHP 2. require_once() -:1676
Notice: Constant REQUIREMENT_INFO already defined in /.../d8/core/includes/install.inc on line 19
Comment | File | Size | Author |
---|---|---|---|
#22 | failures_running-2565971-21.patch | 564 bytes | neclimdul |
#13 | 2565971.13.patch | 1.2 KB | alexpott |
#12 | 2565971.9.patch | 1.19 KB | alexpott |
| |||
#9 | 2565971.9.patch | 1.62 KB | alexpott |
| |||
#3 | failures_running-2565971-3.patch | 496 bytes | neclimdul |
Comments
Comment #2
dawehnerReally interesting because I certainly used
phpunit --filter "UpdateRegistryTest"
only.Why do we even load install.inc twice here, well why even the first time.
Comment #3
neclimdulSo that introduced the failure but the bug was in ModuleHandlerTest which includes install.inc. I haven't looked at the phpunit internals but there is probably some form of fork call that keeps brings along global state. so even when you run in a separate process if someone else pollutes the global space you get a failure.
#2105583: Add some sane strictness to phpunit tests to catch risky tests probably would have caught that but I've been bad and hadn't followed through with it.. :(
Comment #4
dawehnerThis fixes the issue, let's get it in.
On the longrun for those tests we should consider converting them to vfs entirely and empty out install.inc basically.
Comment #5
neclimdulSadly my previous statement seems to be false and that wouldn't have caught it. beStrictAboutChangesToGlobalState only captures globals, super globals and static attributes on classes. defining new constants are not caught.
Comment #7
neclimdulGET http://ec2-52-88-182-40.us-west-2.compute.amazonaws.com/checkout/update.php returned 0 (0 bytes).
Comment #9
alexpottThis breaks my commit workflow... we need to care about the global state. Basically whenever
@runTestsInSeparateProcesses
is used in conjunction with tests that run in the main process we should also do@preserveGlobalState disabled
.Unit tests that load includes not through the autoloader suck.
Comment #11
dawehnermeh
Do we need that part actually? I thought we don't need the isolation as we use vfs only?
Comment #12
alexpottComment #13
alexpottOops... third time lucky.
Comment #15
alexpottFor those wondering why this happens and why #13 is the right fix...
in PHPUnit_Framework_TestCase::run()
Comment #18
dawehnerThank you alex for explaining me what is going on.
Comment #19
catchCommitted/pushed to 8.0.x, thanks!
Comment #20
neclimdulThis is the correct annotation
This was the wrong annotation.
Edit: removed stray sentence that was explained in IRC(and above) prior but was accidentally added..
Comment #22
neclimdulquick follow up.
Comment #23
alexpottMy bad. Sorry @neclimdul
Comment #25
alexpottThe DrupalCI fail is unrelated.
Comment #26
alexpottThanks for catching this @neclimdul. Committed 04b7868 and pushed to 8.0.x. Thanks!