Add methods to cobertura report #655
Conversation
Spacetown
force-pushed
the
add_methods_to_cobertura
branch
from
01290fa
to
8468e4b
Compare
Spacetown
force-pushed
the
add_methods_to_cobertura
branch
from
8468e4b
to
dc0e5c0
Compare
Codecov Report
@@ Coverage Diff @@
## master #655 +/- ##
==========================================
+ Coverage 95.23% 95.29% +0.05%
==========================================
Files 24 24
Lines 3402 3443 +41
Branches 639 647 +8
==========================================
+ Hits 3240 3281 +41
Misses 91 91
Partials 71 71
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
I have two general concerns: method-level line coverage, and the concept of dummy FileCoverage objects.
Attributing lines to functions is impossible for us (right now)
I am worried that the line-to-function attribution might not work in GNU-C or C++ since functions can be nested.
In GNU-C, we can have simple nested functions:
int outer() {
int inner() {
return 40;
}
return inner() + 2;
}In C++11, we'll frequently see lambdas, or other inner classes:
int outer() {
auto inner = []() { return 40; };
return inner() + 2;
}If I read your code correctly, the outer return would be misattributed to the inner function. There is no way for us to fix this without parsing the source code, and we just can't do that. I'd rather provide no data than wrong data (at least by default).
The only way to do this correctly would be to use compiler-level data. Gcov does actually provide the necessary data in its JSON output, which we still aren't parsing 😅
Dummy FileCoverage
This seems like a bit of a code smell. This suggests the need for some kind of RegionCoverage that contains branch/line coverage but does not have a filename. The FileCoverage might inherit from such a RegionCoverage.
| for lineno in function_linenos: | ||
| line_cov = data.lines[lineno] | ||
| dummy_file_coverage.lines[lineno] = line_cov | ||
| if not (line_cov.is_covered or line_cov.is_uncovered): | ||
| continue | ||
|
|
||
| b = line_cov.branch_coverage() | ||
| if b.total: | ||
| function_branch += b | ||
| lines.append(_line_element(line_cov)) |
There was a problem hiding this comment.
This looks like duplicate code to the line element generation below – could probably extracted into a function.
| function_linenos = [] | ||
| for lineno in iter_reversed_sorted_lines: | ||
| if lineno >= function_cov.lineno: | ||
| function_linenos.insert(0, lineno) | ||
| else: | ||
| break |
There was a problem hiding this comment.
This seems to assume that every line belongs to a function, and that there's no code between functions. Perhaps it would be better to avoid making such guesses, and just generate an empty <lines/> element in each method section.
There was a problem hiding this comment.
But then the method element has no information about coverage.
|
@latk You're complete right. Since the methods only make sense with the lines (needed to get the coverage), I think we should close this without merge. |
Oh right. Hmm. This is more difficult than expected. So I think this is blocked on adapting our internal data model to include additional necessary information and implementing a gcov-json reader (#282). That reader should also enable significant performance improvements, and might be able to circumvent the need for the heuristics for finding the working directory. But handling exclude markers + noncode lines would be much more difficult. I think the necessary logic would have to be extracted first from the parser, probably as a function |
As suggested in #654 this PR adds methods to the cobertura report.
The evaluation of the line coverage for functions a dummy
FileCoverageis created to which the lines of the function are added.Closes #654.