From 3695bc5db80d335ebd6e7157e4f2522125f53041 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sat, 6 Jan 2024 14:47:21 +0100 Subject: [PATCH 1/4] Can't have a note inside another tag --- coverage_comment/template_files/comment.md.j2 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/coverage_comment/template_files/comment.md.j2 b/coverage_comment/template_files/comment.md.j2 index 518a3186..6fb2fcfc 100644 --- a/coverage_comment/template_files/comment.md.j2 +++ b/coverage_comment/template_files/comment.md.j2 @@ -244,8 +244,7 @@ _This PR does not seem to contain any modification to coverable code._ {%- if max_files and count_files > max_files %} -> [!NOTE] -> The report is truncated to {{ max_files }} files out of {{ count_files }}. To see the full report, please visit the workflow summary page. +_The report is truncated to {{ max_files }} files out of {{ count_files }}. To see the full report, please visit the workflow summary page._ {% endif %} From d1c133f3d60ef3f088f585f9b8e950c85c620e4c Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sat, 6 Jan 2024 15:21:56 +0100 Subject: [PATCH 2/4] Redo sorting of files to ensure most interesting files come first --- coverage_comment/template.py | 29 +++++++--- tests/unit/test_template.py | 108 +++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 9 deletions(-) diff --git a/coverage_comment/template.py b/coverage_comment/template.py index 7a397878..7d9936fd 100644 --- a/coverage_comment/template.py +++ b/coverage_comment/template.py @@ -108,14 +108,6 @@ class FileInfo: diff: coverage_module.FileDiffCoverage | None previous: coverage_module.FileCoverage | None - @property - def new_missing_lines(self) -> list[int]: - missing_lines = set(self.coverage.missing_lines) - if self.previous: - missing_lines -= set(self.previous.missing_lines) - - return sorted(missing_lines) - def get_comment_markdown( *, @@ -220,12 +212,31 @@ def select_files( files.append(file_info) count_files = len(files) - files = sorted(files, key=lambda x: len(x.new_missing_lines), reverse=True) + files = sorted(files, key=sort_order, reverse=True) if max_files is not None: files = files[:max_files] return sorted(files, key=lambda x: x.path), count_files +def sort_order(file_info: FileInfo) -> tuple[int, int, int]: + """ + Sort order for files: + 1. Files with the most new missing lines + 2. Files with the most added lines (from the diff) + 3. Files with the most new executed lines (including not in the diff) + """ + new_missing_lines = len(file_info.coverage.missing_lines) + if file_info.previous: + new_missing_lines -= len(file_info.previous.missing_lines) + + added_statements = len(file_info.diff.added_statements) if file_info.diff else 0 + new_covered_lines = len(file_info.coverage.executed_lines) + if file_info.previous: + new_covered_lines -= len(file_info.previous.executed_lines) + + return abs(new_missing_lines), added_statements, abs(new_covered_lines) + + def get_readme_markdown( is_public: bool, readme_url: str, diff --git a/tests/unit/test_template.py b/tests/unit/test_template.py index 51880ec2..1e60c18c 100644 --- a/tests/unit/test_template.py +++ b/tests/unit/test_template.py @@ -639,6 +639,30 @@ def test_get_marker(marker_id, result): 3, id="truncated_and_ordered", ), + pytest.param( + """ + # file: a.py + 1 covered + # file: c.py + 1 covered + # file: b.py + 1 covered + """, + """ + # file: a.py + 1 + 2 covered + # file: c.py + + 1 covered + # file: b.py + 1 covered + 1 covered + """, + 2, + ["b.py", "c.py"], + 2, + id="truncated_and_ordered_sort_order_advanced", + ), pytest.param( """ # file: a.py @@ -703,6 +727,90 @@ def test_select_files__no_previous( assert total == 1 +@pytest.mark.parametrize( + "previous_code, current_code_and_diff, expected_new_missing, expected_added, expected_new_covered", + [ + pytest.param( + """ + # file: a.py + 1 covered + 2 missing + """, + """ + # file: a.py + + 1 + 2 covered + + 3 missing + + 4 missing + + 5 covered + """, + 1, + 3, + 1, + id="added_code", + ), + pytest.param( + """ + # file: a.py + 1 covered + 2 missing + 3 covered + 4 missing + """, + """ + # file: a.py + + 1 missing + """, + 1, + 1, + 2, + id="removed_code", + ), + ], +) +def test_sort_order( + make_coverage_and_diff, + make_coverage, + previous_code, + current_code_and_diff, + expected_new_missing, + expected_added, + expected_new_covered, +): + previous_cov = make_coverage(previous_code) + cov, diff_cov = make_coverage_and_diff(current_code_and_diff) + path = pathlib.Path("a.py") + file_info = template.FileInfo( + path=path, + coverage=cov.files[path], + diff=diff_cov.files[path], + previous=previous_cov.files[path], + ) + new_missing, added, new_covered = template.sort_order(file_info=file_info) + assert new_missing == expected_new_missing + assert added == expected_added + assert new_covered == expected_new_covered + + +def test_sort_order__none(make_coverage): + cov = make_coverage( + """ + # file: a.py + 1 covered + """ + ) + file_info = template.FileInfo( + path=pathlib.Path("a.py"), + coverage=cov.files[pathlib.Path("a.py")], + diff=None, + previous=None, + ) + new_missing, added, new_covered = template.sort_order(file_info=file_info) + assert new_missing == 0 + assert added == 0 + assert new_covered == 1 + + def test_get_readme_markdown(): result = template.get_readme_markdown( is_public=True, From e0c33e9055e2a0f3cc3120322943aefe4ab9c9dc Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sat, 6 Jan 2024 15:27:31 +0100 Subject: [PATCH 3/4] Exclude files that have diff that isn't interesting for coverage --- coverage_comment/template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coverage_comment/template.py b/coverage_comment/template.py index 7d9936fd..7f682aab 100644 --- a/coverage_comment/template.py +++ b/coverage_comment/template.py @@ -201,7 +201,7 @@ def select_files( diff=diff_coverage_file, previous=previous_coverage_file, ) - has_diff = bool(diff_coverage_file) + has_diff = bool(diff_coverage_file and diff_coverage_file.added_statements) has_evolution_from_previous = ( previous_coverage_file.info != coverage_file.info if previous_coverage_file From e8869624427012d1c0a087af2e52924175a6f252 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sat, 6 Jan 2024 15:42:53 +0100 Subject: [PATCH 4/4] Fix bug in tests --- tests/conftest.py | 9 +++++++-- tests/unit/test_diff_grouper.py | 7 +++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index c7f44601..7aead5fe 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -291,10 +291,13 @@ def _(code: str, has_branches: bool = True) -> coverage_module.Coverage: ) line_number = 0 # (we start at 0 because the first line will be empty for readabilty) - for line in code.splitlines()[1:]: + for line in code.splitlines(): line = line.strip() + if not line: + continue if line.startswith("# file: "): current_file = pathlib.Path(line.split("# file: ")[1]) + line_number = 0 continue assert current_file, (line, current_file, code) line_number += 1 @@ -383,8 +386,10 @@ def _(code: str) -> tuple[coverage_module.Coverage, coverage_module.DiffCoverage current_file = None # (we start at 0 because the first line will be empty for readabilty) line_number = 0 - for line in code.splitlines()[1:]: + for line in code.splitlines(): line = line.strip() + if not line: + continue if line.startswith("# file: "): new_code += line + "\n" current_file = pathlib.Path(line.split("# file: ")[1]) diff --git a/tests/unit/test_diff_grouper.py b/tests/unit/test_diff_grouper.py index 404d56c7..2050f9d6 100644 --- a/tests/unit/test_diff_grouper.py +++ b/tests/unit/test_diff_grouper.py @@ -24,8 +24,7 @@ def test_group_annotations_more_files( ) assert list(result) == [ - groups.Group(file=pathlib.Path("codebase/code.py"), line_start=6, line_end=8), - groups.Group( - file=pathlib.Path("codebase/other.py"), line_start=17, line_end=17 - ), + groups.Group(file=pathlib.Path("codebase/code.py"), line_start=5, line_end=8), + groups.Group(file=pathlib.Path("codebase/other.py"), line_start=1, line_end=1), + groups.Group(file=pathlib.Path("codebase/other.py"), line_start=3, line_end=5), ]