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):