-
-
Notifications
You must be signed in to change notification settings - Fork 377
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
Line Coverage: mark each line of the Branch as executable #959
Comments
The problem seems to be caused by opcache optimization - php/php-src#9895 - as some expressions can be evaluated on compile time (and thus some expressions are not executed on runtime them). @bwoebi from php-src team advised to disable such optimization. @Slamdunk can you test if all related problems are gone then? ie. for all non-present lines (always const expr) and unstable (const expr. if class was loaded before - #889). For xdebug changes discussion please use https://bugs.xdebug.org/view.php?id=2137 ticket. |
@mvorisek sorry if it's a silly question, but by that do you mean that you need to disable opcache to get a more accurate coverage? |
Not OpCache as a whole but the (optional) bytecode optimizations it can perform. |
Yes, the raw/xdebug coverage /wo opcache is exact/complete (untested, read whole post) - as opcache optimizes some lines out - see https://stackoverflow.com/questions/21181045/php-opcache-optimization-levels-what-are-they to have an idea what opcache optimization are done. Currently, there are two kind of expressions that do not produce coverage lines:
Disabling opcache completely, outside performance, can have side effects like php/php-src#7563 thus it is not a complete solution, however disabling some const expr./dead code elimination optimizations (via |
Fixed in #964 |
In the past year I had deluded myself that an ever curated
ExecutableLinesFindingVisitor
could eventually reach an acceptable quality over what to consider effectively an exec line or not.That will never be the case: xDebug and PCOV are not AST-based, so neither
ExecutableLinesFindingVisitor
can be so.This is even more problematic considering that PHP/xDebug/PCOV just lie.
Take this example, where no bugs of
php-code-coverage
is involved:Lines
12
,13
,14
,15
and16
have been executed for sure by the PHP engine, but the CC driver doesn't consider them as executed.A solution can be to go the other way around of what we've done so far: consider each line of a branch as executable.
Disclaimer: I'm not talking about Branch/Patch coverage provided by xDebug with
XDEBUG_CC_BRANCH_CHECK
. That's a good feature, but pure line coverage is still the fastest and most supported CC methodology available everywhere, so that's where I want our attention to focus on.The idea is: treat every line as executable, split them each into their branch of execution, and consider all lines of that branch as green if any of them is green according to xDebug/PCov.
Let's see this example:
A run with
xdebug_start_code_coverage(XDEBUG_CC_UNUSED|XDEBUG_CC_DEAD_CODE)
gives this result:The AST-based coverage is complex, but there are only 2 branches here, really easy to spot: outside and inside the
if
.Knowing this, we can run this short checklist:
1
outside theif
? Yes; then all the lines of that branch are executed1
inside theif
? No; then no lines of that branch are executedThe final report can be reported as:
The advantages of this approach are:
ExecutableLinesFindingVisitor
get workarounded)/cc @krakjoe @derickr @mvorisek @theofidry
The text was updated successfully, but these errors were encountered: