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

HtmlGenerator: Not loaded source files was not counted to coverage #248

Merged
merged 1 commit into from Nov 14, 2015

Conversation

@jkuchar
Copy link
Contributor

jkuchar commented Aug 29, 2015

Source files that was not loaded when executing tests was not counted to coverage. This produces false results.

I've added estimator of executable lines. What do you think about this prototype? I'm not sure about duplicating code between CloverXMLGenerator and HtmlGenerator. PhpParser can provide quite accurate data for estimate of lines that can be estimated.

@milo

This comment has been minimized.

Copy link
Member

milo commented Aug 31, 2015

Imho, estimation can be more rough. It never will be precise anyway. What about only removing empty lines and count the rest? It will be very pessimistic, but imho OK for this purpose.

@jkuchar

This comment has been minimized.

Copy link
Contributor Author

jkuchar commented Aug 31, 2015

I've done exactly this in first prototype. Removing comment blocks and counting lines gives ~2x times more lines then this filtered version. This gives developer motivation to have at least some test for each file. Because even loading that file will increase coverage. :) Do you like counting lines with PhpParser more or with regex I've proposed?

Or what about counting only functions/methods lines count returned by PhpParser?

@milo

This comment has been minimized.

Copy link
Member

milo commented Sep 17, 2015

PhpParser is overkill here. Imho, this regexp too (it's not 100% correct). Just remove empty lines, it will be super pessimistic, but who cares in this case.

@jkuchar

This comment has been minimized.

Copy link
Contributor Author

jkuchar commented Sep 17, 2015

:+1 (will fix this tomorrow)

@dg dg force-pushed the nette:master branch from 2dbfc68 to eb4ab0f Oct 1, 2015
@jkuchar jkuchar force-pushed the jkuchar:fix-include-all-src-files-into-coverage branch 2 times, most recently from 28b2795 to 09a4a3d Oct 21, 2015
@jkuchar

This comment has been minimized.

Copy link
Contributor Author

jkuchar commented Oct 21, 2015

@milo fixed and rebased onto master

private function getNumberOfNonEmptyLines($file)
{
$lines = 0;
foreach(explode("\n", file_get_contents($file)) as $line) {

This comment has been minimized.

Copy link
@dg

dg Oct 21, 2015

Member

This will not work with \r\n. What about count(file($file, FILE_SKIP_EMPTY_LINES))

This comment has been minimized.

Copy link
@jkuchar

jkuchar Oct 21, 2015

Author Contributor

Thanks! I didn't know about that one. Fixed in cebe26d

@jkuchar jkuchar force-pushed the jkuchar:fix-include-all-src-files-into-coverage branch 2 times, most recently from 2b6cf06 to cebe26d Oct 21, 2015
@jkuchar jkuchar force-pushed the jkuchar:fix-include-all-src-files-into-coverage branch from cebe26d to 3815b58 Oct 22, 2015
@jkuchar

This comment has been minimized.

Copy link
Contributor Author

jkuchar commented Oct 22, 2015

@dg removed comment

@jkuchar

This comment has been minimized.

Copy link
Contributor Author

jkuchar commented Nov 14, 2015

@milo is there anything more to do with this PR?

milo added a commit that referenced this pull request Nov 14, 2015
…overage

HtmlGenerator: Not loaded source files was not counted to coverage
@milo milo merged commit 5f7a775 into nette:master Nov 14, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@milo

This comment has been minimized.

Copy link
Member

milo commented Nov 14, 2015

Thank you. I missed the renewed commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.