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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable xdebug manually #5135

Closed
wants to merge 1 commit into from
Closed

Enable xdebug manually #5135

wants to merge 1 commit into from

Conversation

uuf6429
Copy link
Contributor

@uuf6429 uuf6429 commented Jan 15, 2023

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

Originally posted by @remicollet in #3506 (comment)

It is very typical for developers to run PHP without xdebug and only enable it when necessary through -d zend_extension=xdebug.
PhpStorm, for example follows this approach once properly configured: running tests does not enable xdebug. Debugging or covering tests enables xdebug.

This however does not work for process-isolated-tests (@runTestsInSeparateProcess): even though settings are now copied to the child process, xdebug will not be enabled. This is understandable since it's not trivial to find if xdebug is enabled by default (and I was also not able to find the location of the library programmatically, so PR comment below).

The objective of this PR is to do exactly that. 馃檹


@sebastianbergmann as soon as it's decided where to go, I can also open PRs for other versions.

@@ -41,7 +41,7 @@
"sebastian/code-unit": "^1.0.6",
"sebastian/comparator": "^4.0.8",
"sebastian/diff": "^4.0.3",
"sebastian/environment": "^5.1.3",
"sebastian/environment": "^5.1.4",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming this PR will be released as that version.

@@ -209,6 +209,10 @@ public function getCommand(array $settings, string $file = null): string
array_keys(ini_get_all('xdebug'))
)
);

if (!$this->runtime->isXdebugLoadedByDefault()) {
$settings[] = 'zend_extension=' . (getenv('TODO_USE_PHPUNIT_SETTINGS') ?: 'php_xdebug');
Copy link
Contributor Author

@uuf6429 uuf6429 Jan 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getenv('TODO_USE_PHPUNIT_SETTINGS')

Should I...

  1. ...just use php_xdebug since it already solves most cases
  2. ...use getenv('XX') ?: 'php_xdebug' to offer a bit of flexibility (what should it be named?)
  3. ...use PHPUnit xml config functionality to "do it properly"

@codecov
Copy link

codecov bot commented Jan 15, 2023

Codecov Report

Merging #5135 (1c589c3) into 9.5 (954ca31) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##                9.5    #5135      +/-   ##
============================================
- Coverage     83.39%   83.38%   -0.02%     
- Complexity     4664     4666       +2     
============================================
  Files           275      275              
  Lines         13819    13821       +2     
============================================
  Hits          11524    11524              
- Misses         2295     2297       +2     
Impacted Files Coverage 螖
src/Util/PHP/AbstractPhpProcess.php 68.88% <0.00%> (-0.78%) 猬囷笍

馃摚 We鈥檙e building smart automated test selection to slash your CI/CD build times. Learn more

@sebastianbergmann
Copy link
Owner

Thank you for your contribution. I appreciate the time you invested in preparing this pull request. However, I have decided not to merge it. This should be handled outside of PHPUnit, sorry.

@uuf6429
Copy link
Contributor Author

uuf6429 commented Jan 15, 2023

I understand it's a very specific annoyance, but I can't see it solved outside PHPUnit.

Enabling xdebug globally is terrible for day-to-day use (think installing composer packages with xdebug enabled 馃槅).

Additionally, #3506 feels half-baked without this - one could also argue that it was not needed if PHP was properly configured.

I've also tried hackier approaches, by the way, and nothing worked well (<php><ini...>..., modifying PHPUnit at runtime).. so not a lot of options unless I missed something.


Edit: in case someone sees this, I went with this ugly-ish solution for now:

    public static function setUpBeforeClass(): void
    {
        parent::setUpBeforeClass();

        self::isXdebugEnabledAutomatically() or self::enableXdebugForSubProcesses();
    }

    private static function isXdebugEnabledAutomatically(): bool
    {
        $exp = /** @lang PHP */ "var_export(extension_loaded('xdebug'));";
        $cmd = (new Runtime())->getBinary() . ' -r ' . escapeshellarg($exp) . ' 2>&1';

        return str_contains((string)shell_exec($cmd), 'true');
    }

    private static function enableXdebugForSubProcesses(): void
    {
        $runtime = new Runtime();
        // reset 'binary' property
        $ref = new ReflectionProperty($runtime, 'binary');
        $ref->setValue($runtime, null);
        $runtime->getBinary();
        // update it with xdebug extension
        $ref->setValue($runtime, $ref->getValue($runtime) . ' -d zend_extension=php_xdebug');
    }

@uuf6429 uuf6429 deleted the feature/enable-xdebug-in-subprocess branch January 15, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants