-
-
Notifications
You must be signed in to change notification settings - Fork 384
cleaning filename returned wrongly due to an XDebug bug #84
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
Conversation
I have at the moment a similar / same bug with the following line of code: runkit_function_redefine('time', '', 'return TH::time();'); This throws the following error. [exec] Generating code coverage report, this may take a moment.ErrorException (0): file_get_contents(../tests/Test.php(65) : runkit created function): failed to open stream: No such file or directory |
Yes, it's the same problem, my patch should also fix it |
Good to know. I have my own little fix implemented (similar to yours) and it works, but I have to apply it to every installation I make. Did you get any feedback from sebastian? |
No, i'm still waiting... i've applied the patch to all my installations, luckily it's controlled by puppet, so i didn't have major issues with that. |
@@ -456,6 +456,13 @@ protected function applyListsFilter(&$data) | |||
protected function initializeFilesThatAreSeenTheFirstTime($data) | |||
{ | |||
foreach ($data as $file => $lines) { | |||
// Patch for XDebug bug: | |||
// http://bugs.xdebug.org/bug_view_page.php?bug_id=0000331 | |||
preg_match('/.*\.php/', $file, $matches); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That only works for filenames that end in .php
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the intention. Anyway, if you are use to ending the PHP files with another extension, can be added, or even not filtering by any extension.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code should make no assumption on the suffix.
The fix that I implemented checks in PHP_CodeCoverage_Report_HTML_Renderer_File::loadFile if the file exist, otherwise it returns ''. But this is not at all the nicest fix, especially because Code Coverage has to "fix" a XDebug issue. |
Hi all, I propose another fix for this bug here: #89 I tried to improve the current solution in two ways:
|
I quite like your approach. Especially because the bug is fixed in the XDebug driver and not the "core" code. |
XDebug has a bug which is causing the missing of some files in the php-code-coverage.
When there is an assert or an eval, XDebug returns a wrong file name, something like:
/path/path/path/myClass.php(444) : assert code
They have an open bug since 2007 so it seems that is very low priority for them and they don't have intentions to fix it:
http://bugs.xdebug.org/bug_view_page.php?bug_id=0000331
With the current approach of this coverage tool, if it's not a valid file (checked with $this->filter->isFile($file)), is not included in the coverage report and these files wrongly returned by XDebug, therefore, are missing. So this can be considered a bug.
This change just sanitize the file name returned by XDebug in order to consider them as valid so can be added to the coverage report.
I suppose the $this->filter->isFile($file) won't be necessary but i prefer to let you take the decision of its removal because maybe you prefer to keep it for other cases.