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

Support for Path Coverage #380

Closed
mbrodala opened this issue Sep 1, 2015 · 21 comments
Closed

Support for Path Coverage #380

mbrodala opened this issue Sep 1, 2015 · 21 comments

Comments

@mbrodala
Copy link

mbrodala commented Sep 1, 2015

Starting with version 2.3.0 Xdebug supports collecting branch coverage data.

This package and thus phpunit should use this new feature to gather and output branch coverage in addition to the existing sequence/line coverage.

@sebastianbergmann
Copy link
Owner

This is already being worked on in the https://github.com/sebastianbergmann/php-code-coverage/tree/feature/path-coverage branch. However, I don't think this will be finished anytime soon.

@mbrodala
Copy link
Author

mbrodala commented Sep 1, 2015

Good to know that it's already WIP, thanks. :-)

@omid1234
Copy link

@sebastianbergmann Could we expect any progress here implementing the branch coverage and Dead Code metric in PHPUnit? Please, that is so useful for us and increase the report value of the tool a lot:) Thanks in advance 👍

@sebastianbergmann
Copy link
Owner

I lack the time to work on this, sorry.

@dakorpar
Copy link

@sebastianbergmann any status update on this one?

@VincentChataignier
Copy link

@sebastianbergmann Is this issue still relevant ? Can I submit a PR about this ?
@Maks3w I think you have already worked about branch coverage. Do you have code or PR that I can pick up ?

Thanks 😃

@sebastianbergmann
Copy link
Owner

Resource consumption of Xdebug's branch coverage, time and memory usage, is quite high, even for trivial classes and methods. In light of this, I am not sure how useful support for branch coverage in this component would be.

That being said, I would, of course, consider a pull request that implements this.

However, before anyone starts working on an actual implementation: please prototype the visualisation first.

@sebastianbergmann sebastianbergmann changed the title Add branch coverage Support for Path Coverage Nov 27, 2017
@sebastianbergmann
Copy link
Owner

This visualisation proposed by @derickr is a good start.

paths-covered-mockup

However, I am not sure how well this will work for real world code with hundreds or thousands (or even more) possible execution paths.

@TiMESPLiNTER
Copy link

Any updates about path coverage? Either using xdebug or phpdbg?

@adam-rocska
Copy link

Is this a sick funking joke? Since 2015 there's no progress whatsoever? Even version 9's documentations states "The Opcode Coverage, Branch Coverage, and Path Coverage software metrics are not yet supported by php-code-coverage."... shameful.

@dvdoug
Copy link
Contributor

dvdoug commented May 11, 2020

Hi all

I've started work on this, as I think it will be a great enhancement to have available for https://github.com/dvdoug/behat-code-coverage. PHPUnit can then have it for free 😉

For the first stage, I'm going for simplicity. That means no way to show which paths and branches are/are not covered, simply the numbers of those that are and aren't. Fancier reports can always come later.

Sneak peek
image

image

I hope to be in a position to send the first small PR over later this week 🤞

@sebastianbergmann
Copy link
Owner

Thank you, @dvdoug, for working on this. Please know, however, that I may be reluctant to merge any pull request for path coverage immediately. To be honest, this code base is a mess, mostly due to the massive use of arrays. I would really like to refactor the code to use small value objects instead of arrays to hold the code coverage information, even more so considering that with path coverage we will have an additional kind of information. But maybe adding path coverage to the current state as I imagine it to be. Surprise me with your pull request :-)

@dvdoug
Copy link
Contributor

dvdoug commented May 13, 2020

@sebastianbergmann I did briefly consider leaving things as arrays, but considered that keeping them whilst changing the internal data structure would actually be worse as a BC break than just using an object since breakage would be harder to detect. Therefore encapsulation is one of the first things I've done.

Since this change touches pretty much every area of the codebase and the full diff is absolutely huge, I'll be sending this over incrementally as a series of smaller PRs.

@sebastianbergmann
Copy link
Owner

This sounds very promising and I look forward to your pull requests!

@GPHemsley
Copy link

GPHemsley commented Nov 17, 2020

@sebastianbergmann @dvdoug Has any work been done to support enumerating what the branches/paths are, e.g. by using dump_branch_coverage?

Relevant links:
https://derickrethans.nl/path-branch-coverage.html
https://xdebug.org/docs/code_coverage
https://github.com/xdebug/xdebug/blob/master/tests/coverage/dump-branch-coverage.inc

FWIW, I think Perl's Devel::Cover does a good job of displaying this sort of data. Example:
https://perlmaven.com/devel-cover-for-markua-parser

@dvdoug
Copy link
Contributor

dvdoug commented Nov 18, 2020

@GPHemsley #778 added support for identifying the coverage status of each individual branch (or path). It's admittedly a lot more verbose than the output from Derick's dump script though 😅

Unless you're looking for something specific that I'm missing?

@GPHemsley
Copy link

@dvdoug The current output only shows which lines are executed; it doesn't show why. So if you have a single line with many branches, it can be difficult to identify which scenarios are covered and which aren't.

As you say, the current output is quite verbose, which is why I mentioned Perl's Devel::Cover: I think it does a better job of succinctly identifying what's covered and what's not, for both branches and paths (conditions). The screenshots in the linked blog post give a sense of what the output looks like.

@dvdoug
Copy link
Contributor

dvdoug commented Nov 19, 2020

Ah yes, given an example like if ($a || $b || $c), the report will tell you how many of those branches are covered, but not which precise ones.

However, that info isn't available from Xdebug - the start/end opcode ids in the data are not mappable back to the raw source code

@GPHemsley
Copy link

However, that info isn't available from Xdebug - the start/end opcode ids in the data are not mappable back to the raw source code

Oh, I see. I guess I was assuming that it was necessary to draw the graphs in @derickr's blog post, but looking at them again, I see now that they are just another interpretation of the existing line/branch execution.

That being said, I do think there's room for improvement on displaying the data that is currently available. I find the duplicate lines in the paths confusing, and I feel like the inline T/F branch coverage indicator that Devel::Cover uses more intuitive (even if is less detailed).

Thoughts?

@dvdoug
Copy link
Contributor

dvdoug commented Nov 19, 2020

I assume you're talking about https://perlmaven.com/img/markua-parser-branch-coverage.png ? I'm not familiar with the tool, but looking at it, that output section appears to be limited to just literally if/else? I.e. any individual sub conditions inside the if are actually handled by a different report section? It also doesn't show other types of branch at all (e.g. no switch/case, or an iteration (not)happening with a (non-)empty array?

If I've understood correctly, it would be possible to construct a view like that (with some heavy processing) for certain types of structured code, but it would be trivial to produce examples that I don't think could be handled.

e.g. for something like the below, it would be possible to tokenise the code, find the lines with if statements, search for the existence (if any) of corresponding else statements in the subsequent source code and pull stats from the corresponding lines in the coverage data.

if ($a || $b || $c) {
  doThing();
} else {
  doOtherThing();
}

However reformatting the code to the below, it's not obvious to me how it would be possible to reliably determine which branches correspond to which source-code case without the ability to decompile opcodes.

($a || $b || $c) ? doThing() : doOtherThing();

@sebastianbergmann
Copy link
Owner

As interesting as I find this recent discussion between @GPHemsley and @dvdoug, I do not think that this ticket is the right place for it. Please create a new ticket (maybe "Improve path coverage visualization") and take the discussion there. Thanks!

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 a pull request may close this issue.

9 participants