Skip to content

Added a phpdbg based code coverage driver. #360

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

Closed
wants to merge 5 commits into from
Closed

Added a phpdbg based code coverage driver. #360

wants to merge 5 commits into from

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Jul 20, 2015

Uses phpdbg to create code coverage reports, without x-debug.
Phpdbg works on opcode basis therefore code coverage numbers differ in comparison to x-debug.

The driver requires the phpdbg which is shipped with PHP7.
The features required will not be available in phpdbg+PHP5 and those are also not planned to be backported.

This driver takes all files into account which were included by the time $codeCoverage->stop() is called using get_included_files. All the coverage data is extracted from the phpdbgs' oplog. Files which get included after stop is invoked will not be part of the report (neither as covered nor uncovered).

Main selling point for this driver is that it doesn't have "external dependencies" but works with a regular php7 build (which contains tokenizer and phpdbg).

credits to @bwoebi for the phpdbg implementation part and @rdlowrey, @kelunik for discussions arround the topic.

@GrahamCampbell
Copy link
Contributor

Nice!

@staabm
Copy link
Contributor Author

staabm commented Jul 20, 2015

Please do not merge yet, target is still moving

@staabm staabm changed the title Added a phpdbg based code coverage driver. WIP Added a phpdbg based code coverage driver. Jul 20, 2015
Uses phpdbg to create code coverage reports, without x-debug.
Phpdbg works on opcode basis therefore code coverage numbers differ in comparison to x-debug.
@staabm staabm changed the title WIP Added a phpdbg based code coverage driver. Added a phpdbg based code coverage driver. Jul 21, 2015
@staabm staabm changed the title Added a phpdbg based code coverage driver. WIP Added a phpdbg based code coverage driver. Jul 21, 2015
@rdlowrey
Copy link

👍 It's a hassle to install xdebug when the only thing many folks use it for is phpunit code coverage ... the ability to generate code coverage using phpdbg -- bundled with the php distribution -- is a win for everyone.

@GrahamCampbell
Copy link
Contributor

And xdebug isn't updated for php7 yet.

@staabm staabm changed the title WIP Added a phpdbg based code coverage driver. Added a phpdbg based code coverage driver. Jul 22, 2015
@staabm
Copy link
Contributor Author

staabm commented Jul 22, 2015

Here we go. Requires a php7 built from a recent master version so all new functions are available.

Finally moved some of the logic from userland into phpdbg, because more powerfull at that level.

@rdlowrey
Copy link

Can you post a simple example of how you would use this driver on the command line with phpunit?

TODO: Move this check into Environment\Runtime
@sebastianbergmann
Copy link
Owner

@staabm Thank you for your work on this. Please understand that I won't merge this right away. One reason for this is that we need to figure out how to add support for this additional driver to PHPUnit. PHP_CodeCoverage 2.x has two drivers, PHP_CodeCoverage_Driver_Xdebug and PHP_CodeCoverage_Driver_HHVM. The driver is automatically selected based on the runtime.

PHP_CodeCoverage_Driver_HHVM, has been removed from master for PHP_CodeCoverage 3.x because Facebook decided to change the API of HHVM's code coverage functionality to Xdebug's API thus making a separate driver superfluous.

With the addition of PHP_CodeCoverage_Driver_Phpdbg, choosing the right driver might be as simple as looking at PHP_SAPI and check whether or not Xdebug is loaded: use PHP_CodeCoverage_Driver_Xdebug when PHP_SAPI=cli and ext/xdebug is loaded and use PHP_CodeCoverage_Driver_Phpdbg when PHP_SAPI=phpdbg. This would, however, be an automatic, implicit decision. Maybe it makes sense to make it an explicit one and delegate it to the user. For instance by adding a new configuration option.

We also need to account for the fact that in order to use PHP_CodeCoverage_Driver_Phpdbg PHPUnit must be executing using the phpdbg instead of the php binary. This has the potential to be confusing for users.

@bwoebi It is good to see others (besides @derickr and myself) care about code coverage and working on solutions.

While I do see the benefit of having PHP provide support code coverage out-of-the-box (not because installing Xdebug is a hassle but because due to the fact that it is developed alongside the PHP runtime the potential to "run out of sync" with PHP itself is lower -- basically what happened with OpCache) I fear that you are falling into the same trap with phpdbg that @derickr did with Xdebug. Xdebug started as a debugger and then over the years code coverage, tracing, and profiling functionality were added to it. This, basically, created a monolith potentially making it harder and harder to work on one aspect of functionality of Xdebug without impacting other aspects of its functionality. Similarly, phpdbg started as a debugger and now gets code coverage functionality. Wouldn't it be better to have a separate code coverage extension?

I have not looked at phpdbg's code coverage functionality yet. I doubt, however, that it provides the same quality of data that Xdebug provides today after ~ ten years of development. Don't get me wrong, that is to be expected for any new solution and it's okay. For me to include support for phpdbg in PHP_CodeCoverage, though, I must be certain that the data reported is not worse than what Xdebug reports.

I briefly looked at the implementation of PHP_CodeCoverage_Driver_Phpdbg and understand that phpdbg's code coverage functionality uses a different data structure than Xdebug. Is this documented somewhere? Does phpdbg also support branch and path coverage?

As a sidenote, one pain point I currently have with Xdebug is http://bugs.xdebug.org/view.php?id=1144. @derickr will implement this once he has ported Xdebug to PHP 7. It would be great if phpdbg would also support configuring a blacklist or whitelist.

@rdlowrey I fail to see why it would be considered a hassle having to install Xdebug for code coverage. Following that train of thought it is also a hassle to install PHPUnit for testing.

@GrahamCampbell I trust @derickr to have Xdebug ready for PHP 7 by the time PHP 7 is released.

@bwoebi
Copy link
Contributor

bwoebi commented Jul 25, 2015

Actually, phpdbg has had simple tracing functionality from the start (which just is dumping all the executed opcodes into a file or stdout). The only thing which didn't exist, was a simple API exposing it to userland, which I've added after being asked for it. Originally I just intended a per-opcode output (which still is accessible via "functions" => true, "opcodes" => true), but made by-line the default then. Each executed opcode had the number of times it was executed… Now with lines, it's the number of times opcodes on that line were executed.

I'd like to consider phpdbg a bit like a mix between the classical debugger and valgrind. And profiling is not a business of them nor should it be to phpdbg. But tracing (coverage is basically just a trace collapsed into executions-by-line) should be part of phpdbg I think.
What's still on my wishlist is introducing proper callgrind output… Then the feature set should be rather complete. [I just still haven't really understood that callgrind format, even with the docs :x]

phpdbg currently supports a whitelist (which the patch already uses), it's the "files" option in the call to phpdbg_get_executable(). [That function gives you an array with file => array of executable lines information]. Executable means here that a bunch of opcodes like NOP etc. are all excluded.

What I don't have is branch analysis. Because I have no idea what exactly that does and how it works. What does it do? Eliminate if (0)? Code after a return? I have no idea, but IMHO such code ideally just should be properly commented out.
I'm not even sure if something like branch analysis should be the business of a debugger / tracer?

The issue with a separate code coverage extension is that it'd interfere with every other extension or SAPI overloading zend_execute_ex. They couldn't coexist.


The issue with the SAPI; you might want to make it a setting when default cli binary is used to fork itself into phpdbg to run with it and make the decision implicit when PHP_SAPI == "phpdbg" (as XDebug anyway shouldn't be compatible with phpdbg).

@sebastianbergmann
Copy link
Owner

@bwoebi With regard to Branch Coverage / Path Coverage have a look at http://derickrethans.nl/path-branch-coverage.html. This is something else than dead code / unreachable code detection.

@bwoebi
Copy link
Contributor

bwoebi commented Jul 25, 2015

@sebastianbergmann oh, thanks. Well, I'm not sure whether that should be really in the debugger. I'm more happy giving the tools for that out [like exposing the opcodes via a function (there is phpdbg -p, but I'm sure it's more practical as a function)] in order for a library to nicely convert that into a branch analysis with the trace.

@bwoebi
Copy link
Contributor

bwoebi commented Jul 25, 2015

@sebastianbergmann Also, the documentation of the features will be in the manual for time of PHP 7 RC phases, I guess.

@rdlowrey
Copy link

@sebastianbergmann re: hassle of xdebug

It's not a hassle for people like you and I. But the vast majority of PHP users know nothing about (and are frightened by) anything remotely involving compiling or extensions that aren't directly built-in with their PHP distribution.

The ability to obtain code coverage information using tools packaged with the language is a win for everyone that comes at literally zero cost. It's a no-brainer.

$ phpdbg -rr phpunit --coverage-text

This is far better in my humble opinion than installing xdebug for the sole purpose of test coverage.

@GrahamCampbell
Copy link
Contributor

This is far better in my humble opinion than installing xdebug for the sole purpose of test coverage.

I agree.

@staabm
Copy link
Contributor Author

staabm commented Jul 25, 2015

@sebastianbergmann Its totally fine to not merge it until everythings works as expected. I am already preparing a phpunit fork, so people can start playing with new phpdbg driver easily.

This fork is only meant for testing the new driver and will be dropped after things get sorted and merged into upstream.

@sebastianbergmann
Copy link
Owner

A fork of sebastianbergmann/environment should no longer be necessary: sebastianbergmann/environment@4fe0a44

@sebastianbergmann
Copy link
Owner

@bwoebi Re: #360 (comment) "The issue with the SAPI; you might want to make it a setting when default cli binary is used to fork itself into phpdbg to run with it and make the decision implicit ..."

That fork should happen as soon as possible and therefore must be implemented in https://github.com/sebastianbergmann/phpunit/blob/master/phpunit and https://github.com/sebastianbergmann/phpunit/blob/master/build/phar-autoload.php.in. Not sure, though, how to implement the setting. Just a CLI option? Or also in the XML configuration file? Don't want to add to much logic to PHPUnit's bootstrap. We also need to find the PHPDBG binary.

@sebastianbergmann
Copy link
Owner

@staabm Re: #360 (comment) "I am already preparing a phpunit fork, so people can start playing with new phpdbg driver easily."

I weighed the pros and cons of merging this pull request sooner rather than later. I came to the conclusion that it's best for everyone involved to merge it -- and have already done so. Lets see if we can have a PHP_CodeCoverage 2.2.0 release with PHPDBG support as soon as the the implementation in PHPDBG and the driver for PHP_CodeCoverage are no longer moving targets.

@sebastianbergmann sebastianbergmann self-assigned this Jul 26, 2015
@staabm staabm deleted the phpdbg_cc branch July 26, 2015 09:29
@staabm
Copy link
Contributor Author

staabm commented Jul 26, 2015

Thanks!

@aik099
Copy link

aik099 commented Jul 26, 2015

#360 (comment)

@rdlowrey I'm using xDebug for debugging, don't you?

@kelunik
Copy link
Contributor

kelunik commented Jul 26, 2015

Obviously not, but it's hard to debug with xDebug on PHP 7 anyway, because it's not ready yet. ;-)

@aik099
Copy link

aik099 commented Jul 26, 2015

I won't be sacrificing ease of development/debugging in favor making any project, I'm working on, non backwards compatible to PHP 5.x versions by using PHP7-only features any time soon 😄

@bwoebi
Copy link
Contributor

bwoebi commented Jul 26, 2015

@aik099 you can use phpdbg as a debugger too on PHP 5 projects, as long as they run on PHP 7 too. [At least nearly everything running on PHP 5 runs on PHP 7 too.]

@sebastianbergmann huh, that was fast… thanks :-)

@GrahamCampbell
Copy link
Contributor

ou can use phpdbg as a debugger too on PHP 5 projects, as long as they run on PHP 7 too.

You can't use the code coverage stuff though. PHPDBG has new features in the php 7 version not in the php 5 version.

@bwoebi
Copy link
Contributor

bwoebi commented Jul 26, 2015

@GrahamCampbell I meant, you can run the coverage on a PHP 7 version, even if your taget is PHP 5; coverage usually should be similar to what you get with XDebug.

@GrahamCampbell
Copy link
Contributor

Yeh. :)

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.

7 participants