Skip to content

Fix branch coverage rate comparison when used with --branch without actual branches #37

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

Merged
merged 5 commits into from
Mar 26, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/genbadge/utils_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ class CoverageStats(object):
Contains the results from parsing the coverage.xml.
"""
def __init__(self,
branches_covered=None, branches_valid=None,
complexity=None, lines_covered=None, lines_valid=None
branches_covered=None, branches_valid=None, branch_option=None,
complexity=None, lines_covered=None, lines_valid=None,
):
self.complexity = complexity

self.branches_covered = branches_covered
self.branches_valid = branches_valid
self.branch_option = branch_option

self.lines_covered = lines_covered
self.lines_valid = lines_valid
Expand All @@ -40,10 +41,14 @@ def branch_rate(self):
"""
Note: in --no-branch situations, the number of branches is 0.
In that case, the branch rate is 0 in the coverage.xml.
We mimic the behaviour in this field to be consistent.
But in --branch situations without actual branches,
the number of branches is also 0 but the branch rate is 1.
We mimic both behaviours in this field to be consistent.
"""
if self.branches_valid > 0:
return self.branches_covered / self.branches_valid
elif self.branch_option:
return 1
else:
return 0

Expand Down Expand Up @@ -162,6 +167,10 @@ def parse_root(self, root):
branch_rate = float(root.attrib.get('branch-rate'))
line_rate = float(root.attrib.get('line-rate'))

# detect whether the --branch option were set or not
# so CoverageStats knows how to distinguish between them
cov.branch_option = cov.branches_valid > 0 or branch_rate == 1.0
Comment on lines +170 to +172
Copy link
Owner

Choose a reason for hiding this comment

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

(nitpick)
If branch_option is only computed based on the other two fields according to this formula, could we implement it as a @property instead of a field ? That way it would not need to be present in the constructor arguments, nor to be late-set here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, as cov.branch_rate is computed as a @property while the original branch-rate="1" and branches-valid="0" fields from the XML are what we need to discover whether the --branch option was set or not. Wouldn't that require to store the original branch_rate from the XML? As a _branch_rate/_original_branch_rate/_xml_branch_rate field maybe?

Copy link
Owner

Choose a reason for hiding this comment

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

oh I see your point. And I see that CoverageStats was already entirely implemented in this weird "init as none - then fill" pattern. I do not remember where this class was copied from but indeed making it "nice" would require to change all of it, not just this new feature.
Not worth the effort IMO :) let's keep it like this for now.


if not is_close(cov.branch_rate, branch_rate):
raise ValueError("Computed branch rate (%s) is different from the one in the file (%s)"
% (cov.branch_rate, branch_rate))
Expand Down
23 changes: 23 additions & 0 deletions tests/reports/coverage/coverage_branch_without_branches.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" ?>
<coverage version="7.6.1" timestamp="1725577267091" lines-valid="4" lines-covered="3" line-rate="0.75" branches-valid="0" branches-covered="0" branch-rate="1" complexity="0">
<!-- Generated by coverage.py: https://coverage.readthedocs.io/en/7.6.1 -->
<!-- Based on https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd -->
<sources>
<source>/home/marce_geek/UTN/4/Agiles/python-actions-test</source>
</sources>
<packages>
<package name="." line-rate="0.75" branch-rate="1" complexity="0">
<classes>
<class name="mymodule.py" filename="mymodule.py" complexity="0" line-rate="0.75" branch-rate="1">
<methods/>
<lines>
<line number="1" hits="1"/>
<line number="2" hits="1"/>
<line number="5" hits="1"/>
<line number="6" hits="0"/>
</lines>
</class>
</classes>
</package>
</packages>
</coverage>
19 changes: 19 additions & 0 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,25 @@ def test_parse_cov_nobranch_issue_15():
assert res.total_coverage == 100 * res.total_rate


def test_parse_cov_branch_without_branches():
"""Check that we can parse a coverage.xml file successfully with the branch option without actual branches"""
res = parse_cov(str(TESTS_FOLDER / "reports/coverage/coverage_branch_without_branches.xml"))

assert res.branches_valid == 0
assert res.branches_covered == 0
assert res.branch_rate == 1

assert res.lines_valid == 4
assert res.lines_covered == 3
assert res.line_rate == res.lines_covered / res.lines_valid

assert res.branch_coverage == res.branch_rate * 100
assert res.line_coverage == res.line_rate * 100

assert res.total_rate == ((res.lines_covered + res.branches_covered) / (res.branches_valid + res.lines_valid))
assert res.total_coverage == 100 * res.total_rate


def test_parse_flake8():
"""Check that we can parse a coverage.xml file successfully"""
res = get_flake8_stats(str(TESTS_FOLDER / "reports/flake8/flake8stats.txt"))
Expand Down
Loading