diff --git a/README.md b/README.md index 721db82..ab8ed21 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,7 @@ Note: Either `GITHUB_PR_NUMBER` or `GITHUB_REF` is required. `GITHUB_PR_NUMBER` - `ANNOTATE_MISSING_LINES`: Whether to annotate missing lines in the coverage report. Default is False. - `ANNOTATION_TYPE`: The type of annotation to use for missing lines. 'notice' or 'warning' or 'error'. Default is 'warning'. - `MAX_FILES_IN_COMMENT`: The maximum number of files to include in the coverage report comment. Default is 25. +- `SKIP_COVERED_FILES_IN_REPORT`: Skip the files with coverage 100% from the report. Default is True. - `COMPLETE_PROJECT_REPORT`: Whether to include the complete project coverage report in the comment. Default is False. - `COVERAGE_REPORT_URL`: URL of the full coverage report to mention in the comment. - `DEBUG`: Whether to enable debug mode. Default is False. diff --git a/codecov/config.py b/codecov/config.py index 52fc5b3..c3a28c1 100644 --- a/codecov/config.py +++ b/codecov/config.py @@ -53,6 +53,7 @@ class Config: ANNOTATIONS_OUTPUT_PATH: pathlib.Path | None = None ANNOTATIONS_DATA_BRANCH: str | None = None MAX_FILES_IN_COMMENT: int = 25 + SKIP_COVERED_FILES_IN_REPORT: bool = True COMPLETE_PROJECT_REPORT: bool = False COVERAGE_REPORT_URL: str | None = None # Only for debugging, not exposed in the action @@ -87,6 +88,10 @@ def clean_branch_coverage(cls, value: str) -> bool: def clean_complete_project_report(cls, value: str) -> bool: return str_to_bool(value) + @classmethod + def clean_skip_covered_files_in_report(cls, value: str) -> bool: + return str_to_bool(value) + @classmethod def clean_debug(cls, value: str) -> bool: return str_to_bool(value) diff --git a/codecov/coverage/base.py b/codecov/coverage/base.py index 15fda5e..1a17d35 100644 --- a/codecov/coverage/base.py +++ b/codecov/coverage/base.py @@ -101,7 +101,7 @@ def get_coverage_info(self, coverage_path: pathlib.Path) -> Coverage: @abstractmethod def extract_info(self, data: dict) -> Coverage: - raise NotImplementedError + raise NotImplementedError # pragma: no cover def get_diff_coverage_info( # pylint: disable=too-many-locals self, diff --git a/codecov/coverage/pytest.py b/codecov/coverage/pytest.py index 78eed7c..6b2638e 100644 --- a/codecov/coverage/pytest.py +++ b/codecov/coverage/pytest.py @@ -1,12 +1,16 @@ from __future__ import annotations import datetime +import decimal import pathlib from codecov.coverage.base import BaseCoverage, Coverage, CoverageInfo, CoverageMetadata, FileCoverage class PytestCoverage(BaseCoverage): + def _convert_to_decimal(self, value: float, precision: int = 2) -> decimal.Decimal: + return decimal.Decimal(str(float(value) / 100)).quantize(decimal.Decimal(10) ** -precision) + def extract_info(self, data: dict) -> Coverage: """ { @@ -69,7 +73,7 @@ def extract_info(self, data: dict) -> Coverage: info=CoverageInfo( covered_lines=file_data['summary']['covered_lines'], num_statements=file_data['summary']['num_statements'], - percent_covered=file_data['summary']['percent_covered'], + percent_covered=self._convert_to_decimal(file_data['summary']['percent_covered']), percent_covered_display=file_data['summary']['percent_covered_display'], missing_lines=file_data['summary']['missing_lines'], excluded_lines=file_data['summary']['excluded_lines'], @@ -84,7 +88,7 @@ def extract_info(self, data: dict) -> Coverage: info=CoverageInfo( covered_lines=data['totals']['covered_lines'], num_statements=data['totals']['num_statements'], - percent_covered=data['totals']['percent_covered'], + percent_covered=self._convert_to_decimal(data['totals']['percent_covered']), percent_covered_display=data['totals']['percent_covered_display'], missing_lines=data['totals']['missing_lines'], excluded_lines=data['totals']['excluded_lines'], diff --git a/codecov/main.py b/codecov/main.py index 1dccee7..6362646 100644 --- a/codecov/main.py +++ b/codecov/main.py @@ -71,15 +71,16 @@ def _process_pr(self): log.info('Generating comment for PR #%s', self.github.pr_number) marker = template.get_marker(marker_id=self.config.SUBPROJECT_ID) - files_info, count_files, changed_files_info = template.select_changed_files( + files_info, count_files = template.select_changed_files( coverage=self.coverage, diff_coverage=self.diff_coverage, max_files=self.config.MAX_FILES_IN_COMMENT, + skip_covered_files_in_report=self.config.SKIP_COVERED_FILES_IN_REPORT, ) coverage_files_info, count_coverage_files = template.select_files( coverage=self.coverage, - changed_files_info=changed_files_info, max_files=self.config.MAX_FILES_IN_COMMENT - count_files, # Truncate the report to MAX_FILES_IN_COMMENT + skip_covered_files_in_report=self.config.SKIP_COVERED_FILES_IN_REPORT, ) try: comment = template.get_comment_markdown( diff --git a/codecov/template.py b/codecov/template.py index 2fd8e15..30d4198 100644 --- a/codecov/template.py +++ b/codecov/template.py @@ -142,11 +142,11 @@ def get_comment_markdown( # pylint: disable=too-many-arguments,too-many-locals def select_changed_files( - *, coverage: Coverage, diff_coverage: DiffCoverage, max_files: int | None, -) -> tuple[list[FileInfo], int, list[FileInfo]]: + skip_covered_files_in_report: bool, +) -> tuple[list[FileInfo], int]: """ Selects the MAX_FILES files with the most new missing lines sorted by path These are the files which have been modified in the PR @@ -156,44 +156,42 @@ def select_changed_files( files = [] for path, coverage_file in coverage.files.items(): diff_coverage_file = diff_coverage.files.get(path) + if not (diff_coverage_file and diff_coverage_file.added_statements): + continue + + if skip_covered_files_in_report and percentage_value(diff_coverage_file.percent_covered) == 100: + continue file_info = FileInfo( path=path, coverage=coverage_file, diff=diff_coverage_file, ) - has_diff = bool(diff_coverage_file and diff_coverage_file.added_statements) - - if has_diff: - files.append(file_info) + files.append(file_info) - return sort_and_trucate_files(files=files, max_files=max_files), len(files), files + return sort_and_trucate_files(files=files, max_files=max_files), len(files) def select_files( - *, coverage: Coverage, - changed_files_info: list[FileInfo], max_files: int | None, + skip_covered_files_in_report: bool, ) -> tuple[list[FileInfo], int]: """ Selects the no of `max_files` files from the whole project coverage - Selects only files which are not in changed files report Select only files which have statements (not empty files) + Select only files which have lines missing coverage if `skip_covered_files_in_report` is True """ files = [] - changed_files_path = {file.path for file in changed_files_info} for path, coverage_file in coverage.files.items(): - # Don't show the report for files that have been modified in the PR - # This is gonne be covered in the changed files report - if path in changed_files_path: - continue - # Don't show the report for files that have no statements if coverage_file.info.num_statements == 0: continue + if skip_covered_files_in_report and percentage_value(coverage_file.info.percent_covered) == 100: + continue + file_info = FileInfo(path=path, coverage=coverage_file, diff=None) files.append(file_info) diff --git a/codecov/template_files/comment.md.j2 b/codecov/template_files/comment.md.j2 index c7ca538..b98d362 100644 --- a/codecov/template_files/comment.md.j2 +++ b/codecov/template_files/comment.md.j2 @@ -5,7 +5,7 @@ {%- block coverage_evolution_badge -%} {%- if coverage %} {%- set text = "Coverage of the whole project for this PR is " ~ coverage.info.percent_covered_display ~ "%." -%} -{%- set color = coverage.info.percent_covered | get_badge_color -%} +{%- set color = coverage.info.percent_covered | x100 | get_badge_color -%} {%- endif -%} {%- endblock coverage_evolution_badge -%} diff --git a/codecov/template_files/macros.md.j2 b/codecov/template_files/macros.md.j2 index a1182a5..cd31927 100644 --- a/codecov/template_files/macros.md.j2 +++ b/codecov/template_files/macros.md.j2 @@ -34,7 +34,7 @@ {%- set text = "The coverage rate of " ~ path ~ " is " ~ percent_covered_display ~ "% (" ~ covered_statements_count ~ "/" ~ statements_count ~ ")." -%} {%- set label = percent_covered_display ~ "%" -%} {%- set message = "(" ~ covered_statements_count ~ "/" ~ statements_count ~ ")" -%} -{%- set color = percent_covered | get_badge_color -%} +{%- set color = percent_covered | x100 | get_badge_color -%} {%- endmacro -%} diff --git a/codecov/template_files/pr.md.j2 b/codecov/template_files/pr.md.j2 index c77688b..6c00d84 100644 --- a/codecov/template_files/pr.md.j2 +++ b/codecov/template_files/pr.md.j2 @@ -1,6 +1,6 @@ {%- if not files %} -_This PR does not seem to contain any modification to coverable code._ +_This PR does not include changes to coverable code or code with missing coverage._ {%- else -%}
Click to see coverage of changed files
diff --git a/tests/coverage/test_pytest.py b/tests/coverage/test_pytest.py index cad4ae4..c9b8c11 100644 --- a/tests/coverage/test_pytest.py +++ b/tests/coverage/test_pytest.py @@ -24,7 +24,7 @@ def test_extract_info(self, coverage_json): info=CoverageInfo( covered_lines=6, num_statements=10, - percent_covered=60.0, + percent_covered=PytestCoverage()._convert_to_decimal(60.0), percent_covered_display='60%', missing_lines=4, excluded_lines=0, @@ -40,7 +40,7 @@ def test_extract_info(self, coverage_json): info=CoverageInfo( covered_lines=6, num_statements=10, - percent_covered=60.0, + percent_covered=PytestCoverage()._convert_to_decimal(60.0), percent_covered_display='60%', missing_lines=4, excluded_lines=0, diff --git a/tests/test_config.py b/tests/test_config.py index 896ea82..919d713 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -149,6 +149,11 @@ def test_config_clean_complete_project_report(): assert value is True +def test_config_clean_skip_covered_files_in_report(): + value = config.Config.clean_skip_covered_files_in_report('True') + assert value is True + + def test_config_clean_debug(): value = config.Config.clean_debug('False') assert value is False diff --git a/tests/test_github.py b/tests/test_github.py index 9e8958b..00416e5 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -361,6 +361,11 @@ def test_post_comment_update( test_config, gh_client, ): + comment_with_no_marker = { + 'user': {'login': 'foo'}, + 'body': 'Hey! Hi! How are you?', + 'id': 123, + } comment = { 'user': {'login': 'foo'}, 'body': 'Hey! Hi! How are you? marker', @@ -368,7 +373,7 @@ def test_post_comment_update( } session.register( 'GET', f'/repos/{test_config.GITHUB_REPOSITORY}/issues/{test_config.GITHUB_PR_NUMBER}/comments' - )(json=[comment]) + )(json=[comment_with_no_marker, comment]) session.register( 'PATCH', f'/repos/{test_config.GITHUB_REPOSITORY}/issues/comments/{comment["id"]}', diff --git a/tests/test_template.py b/tests/test_template.py index e00dbd0..54eab9b 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -111,10 +111,11 @@ def test_template_error(coverage_obj, diff_coverage_obj): def test_get_comment_markdown(coverage_obj, diff_coverage_obj): - chaned_files, total, files = template.select_changed_files( + chaned_files, total = template.select_changed_files( coverage=coverage_obj, diff_coverage=diff_coverage_obj, max_files=25, + skip_covered_files_in_report=True, ) result = ( template.get_comment_markdown( @@ -122,7 +123,7 @@ def test_get_comment_markdown(coverage_obj, diff_coverage_obj): diff_coverage=diff_coverage_obj, coverage_files=chaned_files, count_coverage_files=total, - files=files, + files=chaned_files, count_files=total, max_files=25, minimum_green=decimal.Decimal('100'), @@ -153,17 +154,18 @@ def test_get_comment_markdown(coverage_obj, diff_coverage_obj): def test_comment_template(coverage_obj, diff_coverage_obj): - chaned_files, total, files = template.select_changed_files( + chaned_files, total = template.select_changed_files( coverage=coverage_obj, diff_coverage=diff_coverage_obj, max_files=25, + skip_covered_files_in_report=True, ) result = template.get_comment_markdown( coverage=coverage_obj, diff_coverage=diff_coverage_obj, coverage_files=chaned_files, count_coverage_files=total, - files=files, + files=chaned_files, count_files=total, max_files=25, minimum_green=decimal.Decimal('100'), @@ -179,17 +181,18 @@ def test_comment_template(coverage_obj, diff_coverage_obj): def test_comment_template_branch_coverage(coverage_obj, diff_coverage_obj): - chaned_files, total, files = template.select_changed_files( + chaned_files, total = template.select_changed_files( coverage=coverage_obj, diff_coverage=diff_coverage_obj, max_files=25, + skip_covered_files_in_report=True, ) result = template.get_comment_markdown( coverage=coverage_obj, diff_coverage=diff_coverage_obj, coverage_files=chaned_files, count_coverage_files=total, - files=files, + files=chaned_files, count_files=total, max_files=25, minimum_green=decimal.Decimal('100'), @@ -233,13 +236,13 @@ def test_template_no_files(coverage_obj): marker='', subproject_id='foo', ) - assert '_This PR does not seem to contain any modification to coverable code.' in result + assert '_This PR does not include changes to coverable code or code with missing coverage.' in result assert 'code.py' not in result assert 'other.py' not in result @pytest.mark.parametrize( - 'current_code_and_diff, max_files, expected_files, expected_total, expected_total_files', + 'current_code_and_diff, max_files, expected_files, expected_total, covered_skipped_expected_files, covered_skipped_expected_total', [ pytest.param( """ @@ -250,6 +253,7 @@ def test_template_no_files(coverage_obj): [], 0, [], + 0, id='unmodified', ), pytest.param( @@ -262,6 +266,7 @@ def test_template_no_files(coverage_obj): [], 0, [], + 0, id='info_did_not_change', ), pytest.param( @@ -273,6 +278,7 @@ def test_template_no_files(coverage_obj): [], 0, [], + 0, id='info_did_change', ), pytest.param( @@ -283,7 +289,8 @@ def test_template_no_files(coverage_obj): 2, ['a.py'], 1, - ['a.py'], + [], + 0, id='with_diff', ), pytest.param( @@ -296,7 +303,8 @@ def test_template_no_files(coverage_obj): 2, ['a.py', 'b.py'], 2, - ['a.py', 'b.py'], + [], + 0, id='ordered', ), pytest.param( @@ -310,7 +318,8 @@ def test_template_no_files(coverage_obj): 1, ['b.py'], 2, - ['a.py', 'b.py'], + ['b.py'], + 1, id='info_did_change', ), pytest.param( @@ -325,7 +334,8 @@ def test_template_no_files(coverage_obj): 2, ['b.py', 'c.py'], 3, - ['a.py', 'b.py', 'c.py'], + ['b.py', 'c.py'], + 2, id='truncated_and_ordered', ), pytest.param( @@ -342,7 +352,8 @@ def test_template_no_files(coverage_obj): 2, ['b.py', 'c.py'], 2, - ['b.py', 'c.py'], + [], + 0, id='truncated_and_ordered_sort_order_advanced', ), pytest.param( @@ -356,7 +367,8 @@ def test_template_no_files(coverage_obj): None, ['a.py', 'b.py'], 2, - ['a.py', 'b.py'], + ['b.py'], + 1, id='max_none', ), ], @@ -367,21 +379,31 @@ def test_select_changed_files( max_files, expected_files, expected_total, - expected_total_files, + covered_skipped_expected_files, + covered_skipped_expected_total, ): cov, diff_cov = make_coverage_and_diff(current_code_and_diff) - files, total, total_files = template.select_changed_files( + files, total = template.select_changed_files( coverage=cov, diff_coverage=diff_cov, max_files=max_files, + skip_covered_files_in_report=False, ) assert [str(e.path) for e in files] == expected_files assert total == expected_total - assert all(str(e.path) in expected_total_files for e in total_files) + + files, total = template.select_changed_files( + coverage=cov, + diff_coverage=diff_cov, + max_files=max_files, + skip_covered_files_in_report=True, + ) + assert [str(e.path) for e in files] == covered_skipped_expected_files + assert total == covered_skipped_expected_total @pytest.mark.parametrize( - 'current_code_and_diff, max_files, expected_files, expected_total', + 'current_code_and_diff, max_files, expected_files, expected_total, covered_skipped_expected_files, covered_skipped_expected_total', [ pytest.param( """ @@ -391,6 +413,8 @@ def test_select_changed_files( 2, ['a.py'], 1, + [], + 0, id='unmodified', ), pytest.param( @@ -402,6 +426,8 @@ def test_select_changed_files( 2, ['a.py'], 1, + [], + 0, id='info_did_not_change', ), pytest.param( @@ -412,6 +438,8 @@ def test_select_changed_files( 2, ['a.py'], 1, + ['a.py'], + 1, id='info_did_change', ), pytest.param( @@ -420,6 +448,8 @@ def test_select_changed_files( + 1 line covered """, 2, + ['a.py'], + 1, [], 0, id='with_diff', @@ -432,6 +462,8 @@ def test_select_changed_files( + 1 line covered """, 2, + ['a.py', 'b.py'], + 2, [], 0, id='ordered', @@ -447,6 +479,8 @@ def test_select_changed_files( """, 2, ['b.py', 'c.py'], + 3, + ['b.py', 'c.py'], 2, id='truncated_and_ordered', ), @@ -463,7 +497,9 @@ def test_select_changed_files( """, 2, ['a.py', 'b.py'], - 2, + 3, + [], + 0, id='truncated_and_ordered_sort_order_advanced', ), pytest.param( @@ -477,41 +513,39 @@ def test_select_changed_files( None, ['a.py', 'b.py'], 2, + ['b.py'], + 1, id='max_none', ), ], ) def test_select_files( - make_coverage_and_diff, + make_coverage, current_code_and_diff, max_files, expected_files, expected_total, + covered_skipped_expected_files, + covered_skipped_expected_total, ): - cov, diff_cov = make_coverage_and_diff(current_code_and_diff) - _, _, changed_files_info = template.select_changed_files(coverage=cov, diff_coverage=diff_cov, max_files=max_files) - files, total = template.select_files( - coverage=cov, - changed_files_info=changed_files_info, - max_files=max_files, - ) + cov = make_coverage(current_code_and_diff) + files, total = template.select_files(coverage=cov, max_files=max_files, skip_covered_files_in_report=False) assert [str(e.path) for e in files] == expected_files assert total == expected_total + files, total = template.select_files(coverage=cov, max_files=max_files, skip_covered_files_in_report=True) + assert [str(e.path) for e in files] == covered_skipped_expected_files + assert total == covered_skipped_expected_total -def test_select_files_no_statements(make_coverage_and_diff): + +def test_select_files_no_statements(make_coverage): code = """ # file: a.py 1 line covered """ - cov, diff_cov = make_coverage_and_diff(code) - _, _, changed_files_info = template.select_changed_files(coverage=cov, diff_coverage=diff_cov, max_files=2) + cov = make_coverage(code) cov.files[pathlib.Path('a.py')].info.num_statements = 0 - files, total = template.select_files( - coverage=cov, - changed_files_info=changed_files_info, - max_files=2, - ) + files, total = template.select_files(coverage=cov, max_files=2, skip_covered_files_in_report=False) assert [str(e.path) for e in files] == [] assert total == 0