From f47fb1d17ce6f23505ea3b465a3232ab899ca292 Mon Sep 17 00:00:00 2001 From: Scott Dugas Date: Wed, 20 May 2026 10:08:18 -0400 Subject: [PATCH] Add branch coverage to our PR coverage annotations The data was already gathered via JaCoCo so this just exposes it in the same way as the line coverage. branch coverage is omitted if the changed lines have no branches. --- build/coverage_annotations.py | 70 +++++++++++-- build/test_coverage_annotations.py | 151 ++++++++++++++++++++++++++++- 2 files changed, 209 insertions(+), 12 deletions(-) diff --git a/build/coverage_annotations.py b/build/coverage_annotations.py index 0ddda24ea9..6d5c888363 100644 --- a/build/coverage_annotations.py +++ b/build/coverage_annotations.py @@ -204,15 +204,67 @@ def compute_changed_lines_coverage(lines, changed_line_numbers): return covered_changed, executable_changed +def compute_file_branch_coverage(lines): + """ + Compute overall branch coverage for a file. + + Returns: + tuple of (covered_branches, total_branches) + """ + covered = sum(l.covered_branches for l in lines) + missed = sum(l.missed_branches for l in lines) + return covered, covered + missed + + +def compute_changed_lines_branch_coverage(lines, changed_line_numbers): + """ + Compute branch coverage for only the changed lines in a file. + + Returns: + tuple of (covered_branches, total_branches) + """ + line_map = {l.line_number: l for l in lines} + covered = 0 + total = 0 + + for line_num in changed_line_numbers: + line = line_map.get(line_num) + if line is not None: + covered += line.covered_branches + total += line.covered_branches + line.missed_branches + + return covered, total + + def format_percentage(covered, total): - """Format a coverage percentage string.""" + """Format a line coverage percentage string.""" if total == 0: return 'N/A (no executable lines)' pct = (covered / total) * 100 return f'{pct:.1f}% ({covered}/{total} lines)' -def format_file_annotation(repo_path, file_coverage, changed_coverage, first_changed_line): +def format_branch_percentage(covered, total): + """Format a branch coverage percentage string.""" + pct = (covered / total) * 100 + return f'{pct:.1f}% ({covered}/{total} branches)' + + +def format_combined_percentage(line_cov, branch_cov): + """ + Format combined line + branch coverage. Branch part is omitted when there + are no branches at all (keeps annotations tidy for files with no + conditional logic). + """ + parts = [format_percentage(*line_cov)] + if branch_cov[1] > 0: + parts.append(format_branch_percentage(*branch_cov)) + return ', '.join(parts) + + +def format_file_annotation(repo_path, file_line_coverage, changed_line_coverage, + file_branch_coverage, changed_branch_coverage, + first_changed_line): """ Format a single GitHub Actions annotation for a file's coverage summary. @@ -221,11 +273,8 @@ def format_file_annotation(repo_path, file_coverage, changed_coverage, first_cha Returns the annotation string, or None if there's nothing to report. """ - file_covered, file_total = file_coverage - changed_covered, changed_total = changed_coverage - - file_pct_str = format_percentage(file_covered, file_total) - changed_pct_str = format_percentage(changed_covered, changed_total) + file_pct_str = format_combined_percentage(file_line_coverage, file_branch_coverage) + changed_pct_str = format_combined_percentage(changed_line_coverage, changed_branch_coverage) annotation_line = max(1, first_changed_line - 1) message = f'File coverage: {file_pct_str} | Changed lines: {changed_pct_str}' @@ -260,10 +309,15 @@ def generate_annotations(coverage_data, diff_data, source_prefixes): file_coverage = compute_file_coverage(lines) changed_coverage = compute_changed_lines_coverage(lines, changed_line_numbers) + file_branch_coverage = compute_file_branch_coverage(lines) + changed_branch_coverage = compute_changed_lines_branch_coverage(lines, changed_line_numbers) first_changed_line = min(changed_line_numbers) annotation = format_file_annotation( - repo_path, file_coverage, changed_coverage, first_changed_line) + repo_path, + file_coverage, changed_coverage, + file_branch_coverage, changed_branch_coverage, + first_changed_line) if annotation: annotations.append(annotation) diff --git a/build/test_coverage_annotations.py b/build/test_coverage_annotations.py index 5ace1c765b..314b9e3c2f 100644 --- a/build/test_coverage_annotations.py +++ b/build/test_coverage_annotations.py @@ -30,8 +30,12 @@ sys.path.insert(0, os.path.dirname(__file__)) from coverage_annotations import ( LineCoverage, + compute_changed_lines_branch_coverage, compute_changed_lines_coverage, + compute_file_branch_coverage, compute_file_coverage, + format_branch_percentage, + format_combined_percentage, format_file_annotation, format_percentage, generate_annotations, @@ -343,11 +347,13 @@ def test_no_executable_lines(self): class TestFormatFileAnnotation(unittest.TestCase): """Tests for format_file_annotation()""" - def test_basic_annotation(self): + def test_basic_annotation_no_branches(self): result = format_file_annotation( 'module/src/main/java/com/example/Foo.java', (8, 10), (2, 3), + (0, 0), + (0, 0), first_changed_line=15, ) self.assertEqual( @@ -356,11 +362,43 @@ def test_basic_annotation(self): 'File coverage: 80.0% (8/10 lines) | Changed lines: 66.7% (2/3 lines)' ) + def test_annotation_with_branches(self): + result = format_file_annotation( + 'module/src/main/java/com/example/Foo.java', + (8, 10), + (2, 3), + (6, 10), + (1, 4), + first_changed_line=15, + ) + self.assertEqual( + result, + '::notice file=module/src/main/java/com/example/Foo.java,line=14::' + 'File coverage: 80.0% (8/10 lines), 60.0% (6/10 branches) | ' + 'Changed lines: 66.7% (2/3 lines), 25.0% (1/4 branches)' + ) + + def test_annotation_omits_branches_when_only_file_has_branches(self): + # File has branches, but changed lines don't touch any + result = format_file_annotation( + 'module/src/main/java/com/example/Foo.java', + (8, 10), + (2, 3), + (6, 10), + (0, 0), + first_changed_line=15, + ) + self.assertIn('80.0% (8/10 lines), 60.0% (6/10 branches)', result) + self.assertIn('Changed lines: 66.7% (2/3 lines)', result) + self.assertNotIn('0/0 branches', result) + def test_annotation_line_does_not_go_below_1(self): result = format_file_annotation( 'module/src/main/java/com/example/Foo.java', (8, 10), (2, 3), + (0, 0), + (0, 0), first_changed_line=1, ) self.assertIn('line=1', result) @@ -370,11 +408,101 @@ def test_no_executable_changed_lines(self): 'module/src/main/java/com/example/Foo.java', (8, 10), (0, 0), + (0, 0), + (0, 0), first_changed_line=5, ) self.assertIn('N/A (no executable lines)', result) +class TestComputeFileBranchCoverage(unittest.TestCase): + """Tests for compute_file_branch_coverage()""" + + def test_mixed_branches(self): + lines = [ + LineCoverage(10, 0, 3, 0, 0), # no branches + LineCoverage(15, 0, 5, 1, 3), # 3 covered, 1 missed + LineCoverage(20, 0, 1, 2, 2), # 2 covered, 2 missed + ] + covered, total = compute_file_branch_coverage(lines) + self.assertEqual(covered, 5) + self.assertEqual(total, 8) + + def test_no_branches(self): + lines = [ + LineCoverage(10, 0, 3, 0, 0), + LineCoverage(11, 0, 2, 0, 0), + ] + covered, total = compute_file_branch_coverage(lines) + self.assertEqual(covered, 0) + self.assertEqual(total, 0) + + def test_all_branches_covered(self): + lines = [LineCoverage(10, 0, 5, 0, 4)] + covered, total = compute_file_branch_coverage(lines) + self.assertEqual(covered, 4) + self.assertEqual(total, 4) + + def test_empty_file(self): + covered, total = compute_file_branch_coverage([]) + self.assertEqual(covered, 0) + self.assertEqual(total, 0) + + +class TestComputeChangedLinesBranchCoverage(unittest.TestCase): + """Tests for compute_changed_lines_branch_coverage()""" + + def test_changed_lines_with_branches(self): + lines = [ + LineCoverage(10, 0, 3, 0, 0), # no branches + LineCoverage(15, 0, 5, 1, 3), # 3 covered, 1 missed + LineCoverage(20, 0, 1, 2, 2), # 2 covered, 2 missed + ] + # Only line 15 is changed + covered, total = compute_changed_lines_branch_coverage(lines, {15}) + self.assertEqual(covered, 3) + self.assertEqual(total, 4) + + def test_changed_lines_without_branches(self): + lines = [LineCoverage(10, 0, 3, 0, 0)] + covered, total = compute_changed_lines_branch_coverage(lines, {10}) + self.assertEqual(covered, 0) + self.assertEqual(total, 0) + + def test_changed_line_not_in_coverage(self): + lines = [LineCoverage(10, 0, 3, 1, 1)] + # Line 5 is changed but has no coverage data (e.g., a comment) + covered, total = compute_changed_lines_branch_coverage(lines, {5}) + self.assertEqual(covered, 0) + self.assertEqual(total, 0) + + +class TestFormatCombinedPercentage(unittest.TestCase): + """Tests for format_combined_percentage()""" + + def test_with_branches(self): + result = format_combined_percentage((4, 8), (3, 5)) + self.assertEqual(result, '50.0% (4/8 lines), 60.0% (3/5 branches)') + + def test_without_branches(self): + result = format_combined_percentage((4, 8), (0, 0)) + self.assertEqual(result, '50.0% (4/8 lines)') + + def test_no_executable_lines(self): + result = format_combined_percentage((0, 0), (0, 0)) + self.assertEqual(result, 'N/A (no executable lines)') + + +class TestFormatBranchPercentage(unittest.TestCase): + """Tests for format_branch_percentage()""" + + def test_basic(self): + self.assertEqual(format_branch_percentage(3, 5), '60.0% (3/5 branches)') + + def test_full(self): + self.assertEqual(format_branch_percentage(4, 4), '100.0% (4/4 branches)') + + class TestGenerateAnnotations(unittest.TestCase): """Tests for generate_annotations()""" @@ -435,9 +563,24 @@ def test_annotation_contains_coverage_data(self): annotations = generate_annotations(self.coverage_data, diff_data, None) self.assertEqual(len(annotations), 1) # File has 8 executable lines, 4 covered -> 50% - self.assertIn('50.0%', annotations[0]) - # Changed lines 12, 13, 14 are all uncovered -> 0% - self.assertIn('0.0%', annotations[0]) + self.assertIn('50.0% (4/8 lines)', annotations[0]) + # File has 4 total branches (line 15: cb=3 mb=1), 3 covered -> 75% + self.assertIn('75.0% (3/4 branches)', annotations[0]) + # Changed lines 12, 13, 14 are all uncovered with no branches -> 0% lines, no branches + self.assertIn('0.0% (0/3 lines)', annotations[0]) + # Changed lines have no branches, so branches segment should be omitted from changed + self.assertNotIn('0/0 branches', annotations[0]) + + def test_annotation_with_branches_on_changed_line(self): + diff_data = { + # Line 15 has cb=3 mb=1 (partial branch coverage) + 'fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/FDBRecordStore.java': {15}, + } + annotations = generate_annotations(self.coverage_data, diff_data, None) + self.assertEqual(len(annotations), 1) + # Changed line 15: 1 covered line, 3/4 branches covered + self.assertIn('100.0% (1/1 lines)', annotations[0]) + self.assertIn('75.0% (3/4 branches)', annotations[0]) class TestEndToEnd(unittest.TestCase):