Problem/Motivation
Using dump() no longer produces output in PHPUnit tests running under PHP 8.3. This applies to tests run in isolation, annotated with @runInSeparateProcess.
Practically, all core Kernel and Functional(Javascript) tests, and all Unit test annotated with @runInSeparateProcess are subject to this.
The reason is explained by this change in PHP: https://github.com/php/php-src/pull/11169, that led to this change in PHPUnit: https://github.com/sebastianbergmann/phpunit/issues/5356.
See more info https://drupal.slack.com/archives/C1BMUQ9U6/p1710853208536789
Proposed resolution
Make TestVarDumper write to STDERR instead of STDOUT: in fact this is used only for debugging and we should not really care. STDERR output is not swallowed by PHPUnit.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3432171
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3432171-dump-no-longer
changes, plain diff MR !7114
Comments
Comment #2
mondrakeComment #3
mondrakeComment #4
mondrakeComment #5
mondrakeComment #6
mondrakeComment #7
mondrakeIf we can accept
dump()to write to STDERR, this seems easy to fix.Comment #9
mondrakeComment #10
smustgrave commentedIf this is just targetting D11, which requires 8.3, don't see an issue with the change.
Comment #11
longwaveThis should be backported given that developers will also run tests on PHP 8.3 on Drupal 10.
Unsure if it's worth a change record, 99% will not notice the difference, the only thing I can see it affecting is if someone is redirecting output in a shell script or similar?
Comment #12
catchI don't think this is worth a change record, if someone turns out to be affected by this, they're unlikely to find out via the change record anyway I would think.
Comment #13
mondrakeLater when we will have let PHPUnit 9 go, we may consider doing this differently, capturing the dump in a cache and actually printing it as part of the end test event, when the test is actually closed. Instead of using the STD streams that (both OUT and ERR), quite understandably, PHPUnit wants to advocate as part of the SUT, and therefore not mix with test code. At that point yes, a CR would do. But, please, PHPUnit 10+… I wouldn’t tamper with long dead listeners any more now.
Comment #14
alexpottCommitted and pushed 80139bdb8e to 11.x and 5e7ac12c4d to 10.3.x. Thanks!
I think we should backport this to 10.2 because it's really annoying and I'm not really sure there is a bc impact. Relying on the output of dump() feels v wrong.
Comment #17
mondrakeFiled #3436029: [PHPUnit 10+ only] Make dump() output in tests no longer use standard streams for follow up.
Comment #19
longwaveAgreed, backported to 10.2.x as this is tests-only and solves a PHP compatibility issue.