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

Command icon 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:

Comments

mondrake created an issue. See original summary.

mondrake’s picture

Priority: Normal » Major
mondrake’s picture

Issue summary: View changes
mondrake’s picture

Issue summary: View changes
mondrake’s picture

Issue summary: View changes
mondrake’s picture

Issue summary: View changes
mondrake’s picture

If we can accept dump() to write to STDERR, this seems easy to fix.

mondrake’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

If this is just targetting D11, which requires 8.3, don't see an issue with the change.

longwave’s picture

This 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?

catch’s picture

I 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.

mondrake’s picture

Later 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.

alexpott’s picture

Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 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.

  • alexpott committed 5e7ac12c on 10.3.x
    Issue #3432171 by mondrake: dump() no longer produces output in PHPUnit...

  • alexpott committed 80139bdb on 11.x
    Issue #3432171 by mondrake: dump() no longer produces output in PHPUnit...
mondrake’s picture

  • longwave committed 321dc38a on 10.2.x authored by alexpott
    Issue #3432171 by mondrake: dump() no longer produces output in PHPUnit...
longwave’s picture

Status: Patch (to be ported) » Fixed

Agreed, backported to 10.2.x as this is tests-only and solves a PHP compatibility issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.