Skip to content

Conversation

@julianseeger
Copy link
Contributor

When / is the only common directory of the coverage report, the report generation fails.
The root dir "/" is trimmed at several functions, what leads to a path like "home/somefile.php" which is of course not found and thus not imported to the coverage report (at least not with coverage-html).

This PR fixes this issue and allows coverage reports with "/" as the only common path of the report.
I hope the tests point out the problem.

Copy link
Owner

Choose a reason for hiding this comment

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

Can we use a stub of PHP_CodeCoverage_Report_Node (using getMockForAbstractClass()) instead of DefaultNode? If not, can you please move the DefaultNode class into a separate file under tests/_files?

@julianseeger
Copy link
Contributor Author

getMockForAbstractClass mocks every method, so it doesn't seem to be possible to use it and still call the original getPath() method. And the "_files" dir is used in the FilterTest so it's some kind of fixture directory for this test. Are you ok with placing it in the same directory as the NodeTest.php?

@julianseeger
Copy link
Contributor Author

fixed

@sebastianbergmann
Copy link
Owner

I can’t automatically merge this pull request. Can you please rebase/fix? Thanks!

@julianseeger
Copy link
Contributor Author

I can’t automatically merge this pull request. Can you please rebase/fix? Thanks!

Rebased. Did it help?

@sebastianbergmann
Copy link
Owner

This is also a bug fix, right? If so, it should go into 2.0.

@julianseeger
Copy link
Contributor Author

It is, but from my point of view, I'm fine with the next master release here.
The scenario is probably rare so I don't feel the need to fix this in 2.0.

sebastianbergmann added a commit that referenced this pull request Apr 27, 2014
@sebastianbergmann sebastianbergmann merged commit 71eedb7 into sebastianbergmann:master Apr 27, 2014
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.

2 participants