paratestphp / paratest

:computer: Parallel testing for PHPUnit
MIT License
2.32k stars 229 forks source link

NullPhpunitPrinter breaks when output buffer is written to in test case #631

Closed PrinsFrank closed 3 years ago

PrinsFrank commented 3 years ago
Q A
ParaTest version 6.3.1
PHPUnit version 9.5.9
PHP version 8.0.9

Summary

Paratest fails on a single test, that is able to run fine in phpunit itself. There is a dataprovider with 1200 routes in there, that are visited by the controllertest, so the test runs for a while.

Current behavior

Paratest fails on a single test.

How to reproduce: command, code and error stack trace

When testing a bunch of controllers, some legacy controllers that set a header and then exit fail in paratest where they pass in phpunit itself. A simplified testcase:

<?php

namespace Tests\Feature;

use PHPUnit\Framework\TestCase;

class FooTest extends TestCase
{
    public function testFoo(): void
    {
        try {
            header('oeaoeabxuroae');
            exit;
        } catch (\Exception $e) {
        }

        static::assertTrue(true);
    }
}

Test runs fine in phpunit:

user@localhost projectroot % phpunit --testsuite feature
Testing started at 22:13 ...
PHPUnit 9.5.4 by Sebastian Bergmann and contributors.

Time: 00:00.034, Memory: 27.10 MB

OK (1 test, 1 assertion)
Process finished with exit code 0

But fails in paratest:

user@localhost projectroot % php -d memory_limit=4G vendor/bin/paratest --testsuite feature
Running phpunit in 16 processes with projectroot/vendor/phpunit/phpunit/phpunit

Configuration read from projectroot/phpunit.xml

.............................................................   61 / 2513 (  2%)
..............
In WorkerCrashedException.php line 36:

  The command "'php' 'projectroot/vendor/phpunit/phpunit/phpunit' '--configuration' 'projectroot/phpunit.xml' '--no-logging' '--no-coverage' '--printer' 'ParaTest\Runners\PH  
  PUnit\Worker\NullPhpunitPrinter' '--log-junit' '/private/var/folders/ms/xx76q36j6wq2wv9vfq9_5x8h0000gn/T/PT_Iuow5m' 'projectroot/tests/Feature/ControllerTest.php'" failed.                                                                      

  Exit Code: 0(OK)                                                                                                                                                                                                                                                                     

  Working directory: projectroot                                                                                                                                                                                                                 

  Output:                                                                                                                                                                                                                                                                              
  ================                                                                                                                                                                                                                                                                     

  Error Output:                                                                                                                                                                                                                                                                        
  ================                                                                                                                                                                                                                                                                     

In ResultPrinter.php line 227:

  Log file /private/var/folders/ms/xx76q36j6wq2wv9vfq9_5x8h0000gn/T/PT_Iuow5m is empty. This means a PHPUnit process has crashed.                                                                                                                                                      
  The process: 'php' 'projectroot/vendor/phpunit/phpunit/phpunit' '--configuration' 'projectroot/phpunit.xml' '--no-logging' '--no-coverage' '--printer' 'ParaTest\Runners\PH  
  PUnit\Worker\NullPhpunitPrinter' '--log-junit' '/private/var/folders/ms/xx76q36j6wq2wv9vfq9_5x8h0000gn/T/PT_Iuow5m' 'projectroot/tests/Feature/ControllerTest.php'                                                                               
  This means a PHPUnit process was unable to run "projectroot/tests/Feature/ControllerTest.php"                                                                                                                                                    

In Reader.php line 45:

  Log file /private/var/folders/ms/xx76q36j6wq2wv9vfq9_5x8h0000gn/T/PT_Iuow5m is empty. This means a PHPUnit process has crashed.  

paratest [--bootstrap BOOTSTRAP] [--colors] [-c|--configuration CONFIGURATION] [--coverage-clover COVERAGE-CLOVER] [--coverage-cobertura COVERAGE-COBERTURA] [--coverage-crap4j COVERAGE-CRAP4J] [--coverage-html COVERAGE-HTML] [--coverage-php COVERAGE-PHP] [--coverage-test-limit COVERAGE-TEST-LIMIT] [--coverage-text [COVERAGE-TEXT]] [--coverage-xml COVERAGE-XML] [--exclude-group EXCLUDE-GROUP] [--filter FILTER] [-f|--functional] [-g|--group GROUP] [-h|--help] [--log-junit LOG-JUNIT] [--log-teamcity LOG-TEAMCITY] [-m|--max-batch-size MAX-BATCH-SIZE] [--no-coverage] [--no-test-tokens] [--order-by [ORDER-BY]] [--parallel-suite] [--passthru PASSTHRU] [--passthru-php PASSTHRU-PHP] [--path PATH] [-p|--processes PROCESSES] [--random-order-seed [RANDOM-ORDER-SEED]] [--repeat [REPEAT]] [--runner RUNNER] [--stop-on-failure] [--testsuite TESTSUITE] [--tmp-dir TMP-DIR] [-v|vv|--verbose] [--whitelist WHITELIST] [--] [<path>]

Expected behavior

The test should pass as it does so in phpunit itself.

Debugging result

Disabling the printer line in ParaTest\Runners\PHPUnit\ExecutableTest makes paratest pass:

    final public function commandArguments(string $binary, array $options, ?array $passthru): array
    {
        $options                = $this->prepareOptions($options);
        $options['no-logging']  = null;
        $options['no-coverage'] = null;
//      $options['printer']     = NullPhpunitPrinter::class;

Diving into ParaTest\Runners\PHPUnit\Worker\NullPhpunitPrinter I can see only one difference with the other result printers, and that is related to output buffering. Changing this single line makes paratest pass as well:

final class NullPhpunitPrinter implements ResultPrinter
{
    use TestListenerDefaultImplementation;

    public function printResult(TestResult $result): void
    {
    }

    public function write(string $buffer): void
    {
+++++ print 1;
    }
}
Slamdunk commented 3 years ago

Hi, the behavior you are describing is expected: you are testing that header() raises an Exception, but this only happens when headers are already sent, which they never are with our NullPrinter.

See:

$ php -r 'header("foo");'

$ php -r 'echo PHP_EOL;header("foo");'

PHP Warning:  Cannot modify header information - headers already sent by (output started at Command line code:1) in Command line code on line 1

In other words, your test suite is coupled with the testing tool, which should never be.

PrinsFrank commented 3 years ago

I am not actually asserting that the exception is thrown. The header call and the exception catching is actually done in code in laravel controllers and middleware in our codebase, but I moved it to the test function to make a minimal reproduction. the assertTrue on true is only to demonstrate a passing test and not make phpunit mark this as risky.

Besides the fact that we shouldn't set headers in controllers this way in laravel - which I have since removed - there does seem to be an issue in the combination of paratest and phpunit;

An even more minimalistic repo (which is marked as risky becaus no assertions are done):

<?php

class FooTest extends PHPUnit\Framework\TestCase
{
    public function testFoo(): void
    {
        try {
            header('oeaoeabxuroae');
            exit;
        } catch (\Exception $e) {
        }
    }
}

I suspect there is an issue in phpunit itself that expects the presenter to write something to the output buffer, which fails when the presenter doesn't write anything but code hit in a test does. I've looked at all 'ob_*' functions in both phpunit and paratest but can't really find any obvious buffering bugs.

The workaround of actually printing/echoing in the NullPhpunitPrinter could be a quick fix so tests that hit code modifying the output buffer can run using paratest. Fixing the original issue in phpunit (And maybe getting the NullPhpunitPrinter into phpunit itself) needs some help of the devs of phpunit.

I've personally spent quite some time on figuring out why my testcase wouldn't run in paratest but succesfully runs in phpunit, so even when this issue is closed without changes it might help the next person.

Slamdunk commented 3 years ago

Let me clarify what's happening in more details in both cases:

  1. vendor/bin/phpunit FooTest.php starts
  2. The string PHPUnit 9.5.8 by Sebastian Bergmann and contributors. is printed to STDOUT: from this point no header() can be called
  3. The test starts: the header() call raises a PHP Warning: Cannot modify header information - headers already sent Exception
  4. The try {} catch {} block skips the exit call and proceeds the test as normal
  5. The test suite ends successfully

With the NullPrinter, even in PHPUnit (so no ParaTest involved):

  1. vendor/bin/phpunit --printer 'ParaTest\Runners\PHPUnit\Worker\NullPhpunitPrinter' FooTest.php starts
  2. STDOUT never receive any stream data
  3. The test starts: the header() call does its job without errors
  4. The test continue: the exit; call exits the entire computer process

As you can see, the test suite is coupled with the testing tool, which should never be. header() is a bad beast.