Skip to content

Commit

Permalink
[lcov] Improve reporting of branch destinations.
Browse files Browse the repository at this point in the history
The branch field of a BRDA: record can be an arbitrary textual label.
Therefore, instead of emitting meaningless numbers, emit the string
“to line <N>” for ordinary branches (where <N> is the arc destination
line, and “to exit” for branches that exit the function.  When there is
more than one exit arc from a single line, provide the negated arc
destination as a disambiguator.

Thanks to Henry Cox (@henry2cox), one of the LCOV maintainers, for
clarifying the semantics of BRDA: records for us.
  • Loading branch information
zackw committed Sep 11, 2024
1 parent 923ba5a commit 583961e
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 41 deletions.
75 changes: 42 additions & 33 deletions coverage/lcovreport.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,41 +64,50 @@ def lcov_arcs(
missing_arcs = analysis.missing_branch_arcs()

for line in lines:
if line in branch_stats:
# The meaning of a BRDA: line is not well explained in the lcov
# documentation. Based on what genhtml does with them, however,
# the interpretation is supposed to be something like this:
# BRDA: <line>, <block>, <branch>, <hit>
# where <line> is the source line number of the *origin* of the
# branch; <block> is an arbitrary number which distinguishes multiple
# control flow operations on a single line; <branch> is an arbitrary
# number which distinguishes the possible destinations of the specific
# control flow operation identified by <line> + <block>; and <hit> is
# either the hit count for <line> + <block> + <branch> or "-" meaning
# that <line> + <block> was never *reached*. <line> must be >= 1,
# and <block>, <branch>, <hit> must be >= 0.

# This is only one possible way to map our sets of executed and
# not-executed arcs to BRDA codes. It seems to produce reasonable
# results when fed through genhtml.

if line not in branch_stats:
continue

# This is only one of several possible ways to map our sets of executed
# and not-executed arcs to BRDA codes. It seems to produce reasonable
# results when fed through genhtml.
_, taken = branch_stats[line]

if taken == 0:
# When _none_ of the out arcs from 'line' were executed,
# this probably means means 'line' was never executed at all.
# Cross-check with the line stats.
assert len(executed_arcs[line]) == 0
if line in analysis.missing:
hit = "-" # indeed, never reached
else:
hit = "0" # I am not prepared to swear this is impossible
destinations = [
(int(dst < 0), abs(dst), hit) for dst in missing_arcs[line]
]
else:
# Q: can we get counts of the number of times each arc was executed?
# branch_stats has "total" and "taken" counts for each branch, but it
# doesn't have "taken" broken down by destination.
destinations = {}
for dst in executed_arcs[line]:
destinations[(int(dst < 0), abs(dst))] = 1
for dst in missing_arcs[line]:
destinations[(int(dst < 0), abs(dst))] = 0

if all(v == 0 for v in destinations.values()):
# When _none_ of the out arcs from 'line' were executed, presume
# 'line' was never reached.
for branch, _ in enumerate(sorted(destinations.keys())):
outfile.write(f"BRDA:{line},0,{branch},-\n")
# branch_stats has "total" and "taken" counts for each branch,
# but it doesn't have "taken" broken down by destination.
destinations = [
(int(dst < 0), abs(dst), "1") for dst in executed_arcs[line]
]
destinations.extend(
(int(dst < 0), abs(dst), "0") for dst in missing_arcs[line]
)

destinations.sort()
n_exits = sum(
1 for is_exit, _, _ in destinations if is_exit
)
for is_exit, dst, hit in destinations:
if is_exit:
if n_exits == 1:
branch = "to exit"
else:
branch = f"to exit {dst}"
else:
for branch, (_, hit) in enumerate(sorted(destinations.items())):
outfile.write(f"BRDA:{line},0,{branch},{hit}\n")
branch = f"to line {dst}"
outfile.write(f"BRDA:{line},0,{branch},{hit}\n")

# Summary of the branch coverage.
brf = sum(t for t, k in branch_stats.values())
Expand Down
83 changes: 75 additions & 8 deletions tests/test_lcov.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ def is_it_x(x):
DA:5,0
LF:4
LH:1
BRDA:2,0,0,-
BRDA:2,0,1,-
BRDA:2,0,to line 3,-
BRDA:2,0,to line 5,-
BRF:2
BRH:0
end_of_record
Expand Down Expand Up @@ -203,8 +203,8 @@ def test_is_it_x(self):
DA:5,0
LF:4
LH:1
BRDA:2,0,0,-
BRDA:2,0,1,-
BRDA:2,0,to line 3,-
BRDA:2,0,to line 5,-
BRF:2
BRH:0
end_of_record
Expand Down Expand Up @@ -247,8 +247,8 @@ def test_half_covered_branch(self) -> None:
DA:6,0
LF:4
LH:3
BRDA:3,0,0,1
BRDA:3,0,1,0
BRDA:3,0,to line 4,1
BRDA:3,0,to line 6,0
BRF:2
BRH:1
end_of_record
Expand Down Expand Up @@ -315,11 +315,78 @@ def test_excluded_lines(self) -> None:
DA:6,1
LF:4
LH:3
BRDA:3,0,0,0
BRDA:3,0,1,1
BRDA:3,0,to line 4,0
BRDA:3,0,to line 6,1
BRF:2
BRH:1
end_of_record
""")
actual_result = self.get_lcov_report_content()
assert expected_result == actual_result

def test_exit_branches(self) -> None:
self.make_file("runme.py", """\
def foo(a):
if a:
print(f"{a!r} is truthy")
foo(True)
foo(False)
foo([])
foo([0])
""")
cov = coverage.Coverage(source=".", branch=True)
self.start_import_stop(cov, "runme")
cov.lcov_report()
expected_result = textwrap.dedent("""\
SF:runme.py
DA:1,1
DA:2,1
DA:3,1
DA:4,1
DA:5,1
DA:6,1
DA:7,1
LF:7
LH:7
BRDA:2,0,to line 3,1
BRDA:2,0,to exit,1
BRF:2
BRH:2
end_of_record
""")
actual_result = self.get_lcov_report_content()
assert expected_result == actual_result

def test_multiple_exit_branches(self) -> None:
self.make_file("runme.py", """\
def foo(a):
if any(x > 0 for x in a):
print(f"{a!r} has positives")
foo([])
foo([0])
foo([0,1])
foo([0,-1])
""")
cov = coverage.Coverage(source=".", branch=True)
self.start_import_stop(cov, "runme")
cov.lcov_report()
expected_result = textwrap.dedent("""\
SF:runme.py
DA:1,1
DA:2,1
DA:3,1
DA:4,1
DA:5,1
DA:6,1
DA:7,1
LF:7
LH:7
BRDA:2,0,to line 3,1
BRDA:2,0,to exit 1,1
BRDA:2,0,to exit 2,1
BRF:2
BRH:2
end_of_record
""")
actual_result = self.get_lcov_report_content()
assert expected_result == actual_result

0 comments on commit 583961e

Please sign in to comment.