Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP options should be passed to child processes #3506

Closed
remicollet opened this issue Feb 1, 2019 · 15 comments
Closed

PHP options should be passed to child processes #3506

remicollet opened this issue Feb 1, 2019 · 15 comments
Labels
type/enhancement A new idea that should be implemented

Comments

@remicollet
Copy link
Contributor

remicollet commented Feb 1, 2019

Q A
PHPUnit version 8.0.0
PHP version 7.2.14
Installation Method Composer

Playing with pcov...

When enabled (in php.ini), everything OK

$ php vendor/bin/phpunit 
PHPUnit 8.0.0 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.2.14 with PCOV 1.0.0
Configuration: /work/GIT/sql-parser/phpunit.xml.dist

When disabled globally, and only enabled in command line, it fails

$ php -d pcov.enabled=1 vendor/bin/phpunit 
PHPUnit 8.0.0 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.2.14 with PCOV 1.0.0
Configuration: /work/GIT/sql-parser/phpunit.xml.dist

.....................................II.................EW.....  63 / 545 ( 11%)

With some errors like: "No code coverage driver available"

1) PhpMyAdmin\SqlParser\Tests\Parser\ComponentTest::testParse
PHPUnit\Framework\Exception: Fatal error: Uncaught SebastianBergmann\CodeCoverage\RuntimeException: No code coverage driver available in /work/GIT/sql-parser/vendor/phpunit/php-code-coverage/src/CodeCoverage.php:898
Stack trace:
#0 /work/GIT/sql-parser/vendor/phpunit/php-code-coverage/src/CodeCoverage.php(147): SebastianBergmann\CodeCoverage\CodeCoverage->selectDriver(Object(SebastianBergmann\CodeCoverage\Filter))
#1 Standard input code(40): SebastianBergmann\CodeCoverage\CodeCoverage->__construct(NULL, Object(SebastianBergmann\CodeCoverage\Filter))
#2 Standard input code(112): __phpunit_run_isolated_test()
#3 {main}
  thrown in /work/GIT/sql-parser/vendor/phpunit/php-code-coverage/src/CodeCoverage.php on line 898

Caused by
ErrorException: unserialize(): Error at offset 0 of 696 bytes in /work/GIT/sql-parser/vendor/phpunit/phpunit/src/Util/PHP/AbstractPhpProcess.php:254
Stack trace:
#0 [internal function]: PHPUnit\Util\PHP\AbstractPhpProcess->PHPUnit\Util\PHP\{closure}(8, 'unserialize(): ...', '/work/GIT/sql-p...', 254, Array)
#1 /work/GIT/sql-parser/vendor/phpunit/phpunit/src/Util/PHP/AbstractPhpProcess.php(254): unserialize('\nFatal error: U...')
#2 /work/GIT/sql-parser/vendor/phpunit/phpunit/src/Util/PHP/AbstractPhpProcess.php(170): PHPUnit\Util\PHP\AbstractPhpProcess->processChildResult(Object(PhpMyAdmin\SqlParser\Tests\Parser\ComponentTest), Object(PHPUnit\Framework\TestResult), '\nFatal error: U...', '')
#3 /work/GIT/sql-parser/vendor/phpunit/phpunit/src/Framework/TestCase.php(795): PHPUnit\Util\PHP\AbstractPhpProcess->runTestJob('<?php\nuse PHPUn...', Object(PhpMyAdmin\SqlParser\Tests\Parser\ComponentTest), Object(PHPUnit\Framework\TestResult))
#4 /work/GIT/sql-parser/vendor/phpunit/phpunit/src/Framework/TestSuite.php(761): PHPUnit\Framework\TestCase->run(Object(PHPUnit\Framework\TestResult))
#5 /work/GIT/sql-parser/vendor/phpunit/phpunit/src/Framework/TestSuite.php(761): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
#6 /work/GIT/sql-parser/vendor/phpunit/phpunit/src/Framework/TestSuite.php(761): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
#7 /work/GIT/sql-parser/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(632): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
#8 /work/GIT/sql-parser/vendor/phpunit/phpunit/src/TextUI/Command.php(208): PHPUnit\TextUI\TestRunner->doRun(Object(PHPUnit\Framework\TestSuite), Array, true)
#9 /work/GIT/sql-parser/vendor/phpunit/phpunit/src/TextUI/Command.php(164): PHPUnit\TextUI\Command->run(Array, true)
#10 /work/GIT/sql-parser/vendor/phpunit/phpunit/phpunit(61): PHPUnit\TextUI\Command::main()
#11 {main}

@remicollet
Copy link
Contributor Author

@krakjoe FYI

@remicollet remicollet changed the title PHP option not passed to inherited process PHP option not inherited in child process Feb 1, 2019
@remicollet remicollet changed the title PHP option not inherited in child process PHP options not inherited in child process Feb 1, 2019
@krakjoe
Copy link
Contributor

krakjoe commented Feb 1, 2019

Maybe related to #3492

@krakjoe
Copy link
Contributor

krakjoe commented Feb 1, 2019

So here's what I got, I think this isn't too bad ...

This in Runtime (but with a better name):

    public function getCurrentSettings(array $values) : array {
        $diff = [];

        $files = [\php_ini_loaded_file()];
        if ($scanned = \php_ini_scanned_files()) {
            $files = \array_merge($files, $scanned);
        }

        foreach ($files as $ini) {
            $config = \parse_ini_file($ini, true);

            foreach ($values as $value) {
                $set = \ini_get($value);

                if (isset($config[$value]) && $set != $config[$value]) {
                    $diff[] = sprintf("%s=%s", $value, $set);
                }
            }
        }

        return $diff;
    }

and this in AbstractPhpProcess::getCommand

    public function getCommand(array $settings, string $file = null): string
    {
        $command = $this->runtime->getBinary();

	if ($this->runtime->hasPCOV()) {
            $pcov = $this->runtime->getCurrentSettings(
		\array_keys(\ini_get_all("pcov")));
            $settings = \array_merge($settings, $pcov);
        } else if ($this->runtime->hasXdebug()) {
            $xdebug = $this->runtime->getCurrentSettings(
                \array_keys(\ini_get_all("xdebug")));
            $settings = \array_merge($settings, $xdebug);
        }

        $command .= $this->settingsToParameters($settings);

This is working for xdebug and pcov ...

Any thoughts ?

EDIT: php_ini_loaded_file may return false, but you get the gist of it ...

@remicollet
Copy link
Contributor Author

remicollet commented Feb 2, 2019

For Pcov, "production" default is PCOV.enabled=0, and why was thinking to use it this way, using the command line option.

For XDebug, this is a bit different, as there is no "enable" option (may have to be enabled using -d zend_extension=xdebug, but this can be another issue because of extension load order).

@krakjoe
Copy link
Contributor

krakjoe commented Feb 2, 2019

There is coverage_enable in xdebug, that's what I tested this approach with but would also work for the case where only loaded with -d

The main thing we want to achieve is to make sure the child process is always run with the settings of the parent and the current approach cannot do that because it cannot access the configuration set with -d.

Looking at it fresh this morning, I'm not sure if a simple array_merge may be what we want, or we may want to merge the passed settings into the system ones the other way round than I did it in the example code ...

@stale
Copy link

stale bot commented Apr 3, 2019

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 3, 2019
@krakjoe
Copy link
Contributor

krakjoe commented Apr 3, 2019

@sebastianbergmann can we get some input on this please ?

@stale stale bot removed the stale label Apr 3, 2019
@sebastianbergmann
Copy link
Owner

Child processes currently do not "inherit" PHP options. They probably should. #3506 (comment) is hard for me to look at / review / judge. A pull request would make it a lot easier for me to review what you propose.

@krakjoe
Copy link
Contributor

krakjoe commented Apr 3, 2019

ack, will do pulls ...

@sebastianbergmann
Copy link
Owner

Thanks! And sorry for dropping the ball on this.

@sebastianbergmann sebastianbergmann added the type/enhancement A new idea that should be implemented label Apr 22, 2019
@sebastianbergmann sebastianbergmann changed the title PHP options not inherited in child process PHP options should be passed to child processes Apr 22, 2019
@sebastianbergmann
Copy link
Owner

@krakjoe Did you get a chance to work on this yet? Thanks!

@christianjbrown
Copy link

Just wanted to double check - this is fixed in PHP Unit 9 because it uses a later version of sebastian/environment that has the fix?

No chance of a fix for 8.5?

@sebastianbergmann
Copy link
Owner

AFAICS, sebastianbergmann/environment@61420aa is in version 4.2.0 and up of this component. And 4.x is what PHPUnit 8.5 uses.

@christianjbrown
Copy link

I think we can confirm this is still occurring in PHPUnit 8.5. Command line PHP options, ie -d options, are not being passed on to tests that have @runInSeparateProcess.

Maybe it's more specifically #3492, the d=xdebug.so is not passing on. Worth reopening that ticket or this more generic one?

@robsontenorio
Copy link

Just opened a new issue. It is not working for me. Could you confirm?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A new idea that should be implemented
Projects
None yet
Development

No branches or pull requests

5 participants