From 96219707391f3ed89e728422177c36978c5e4269 Mon Sep 17 00:00:00 2001 From: Pradeep Tammali Date: Sat, 30 Aug 2025 23:35:07 +0200 Subject: [PATCH] refactor: minor changes --- README.md | 6 ++- codecov/badge.py | 2 + codecov/config.py | 29 +++++++----- codecov/coverage/base.py | 99 ++++++++++++++++++++-------------------- codecov/github.py | 96 ++++++++++++++++++++------------------ codecov/template.py | 3 +- tests/conftest.py | 2 +- tests/test_config.py | 8 ++-- 8 files changed, 130 insertions(+), 115 deletions(-) diff --git a/README.md b/README.md index ab8ed21..b542581 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ # python-coverage-comment +## WIP 🚧 + Create a Coverage report comment on Github PR Permissions needed for the Github Token: @@ -8,7 +10,7 @@ Permissions needed for the Github Token: `Pull requests:write` If you have given `ANNOTATIONS_DATA_BRANCH` branch then Github Token also requires content write permissions. -Read more on how to use this [here](./docs/annotations.md). +Read more on how to use this [document](./docs/annotations.md). `Contents:read` `Contents:write` @@ -46,7 +48,7 @@ Note: Either `GITHUB_PR_NUMBER` or `GITHUB_REF` is required. `GITHUB_PR_NUMBER` - `MINIMUM_ORANGE`: The minimum coverage percentage for orange status. Default is 70. - `BRANCH_COVERAGE`: Show branch coverage in the report. Default is False. - `SKIP_COVERAGE`: Skip coverage reporting as github comment and generate only annotaions. Default is False. -- `ANNOTATIONS_DATA_BRANCH`: The branch to store the annotations. Read more about this [here](./docs/annotations.md). +- `ANNOTATIONS_DATA_BRANCH`: The branch to store the annotations. Read more about this [document](./docs/annotations.md). - `ANNOTATIONS_OUTPUT_PATH`: The path where the annotaions should be stored. Should be a path to folder. - `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'. diff --git a/codecov/badge.py b/codecov/badge.py index 3d3726a..4ad7883 100644 --- a/codecov/badge.py +++ b/codecov/badge.py @@ -29,5 +29,7 @@ def get_static_badge_url(label: str, message: str, color: str) -> str: if not color or not message: log.error('Both "color" and "message" are required to generate the badge URL.') raise ValueError + code = '-'.join(e.replace('_', '__').replace('-', '--') for e in (label, message, color) if e) + # Please read here on how this badge creation works https://shields.io/badges return 'https://img.shields.io/badge/' + urllib.parse.quote(f'{code}.svg') diff --git a/codecov/config.py b/codecov/config.py index e550219..95819c8 100644 --- a/codecov/config.py +++ b/codecov/config.py @@ -11,12 +11,16 @@ from codecov.exceptions import MissingEnvironmentVariable -def path_below(path_str: str | pathlib.Path) -> pathlib.Path: +def _is_json_file(path: pathlib.Path) -> bool: + return path.suffix == '.json' + + +def resolve_path(path_str: str | pathlib.Path) -> pathlib.Path: path = pathlib.Path(path_str).resolve() if not (path.exists() and path.is_file()): raise ValueError('Path does not exist') - if path.suffix != '.json': + if not _is_json_file(path): raise ValueError('The file is not a JSON file.') return path @@ -40,7 +44,7 @@ class Config: COVERAGE_PATH: pathlib.Path GITHUB_TOKEN: str = dataclasses.field(repr=False) GITHUB_PR_NUMBER: int | None = None - # Branch to run the action on (alternate to get PR number if not provided) + # Branch to create the comment on (alternate to get PR number if not provided) # Example Organisation:branch-name (Company:sample-branch) or User:branch-name (user:sample-branch) GITHUB_REF: str | None = None SUBPROJECT_ID: str | None = None @@ -56,7 +60,6 @@ class Config: 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 DEBUG: bool = False def __post_init__(self) -> None: @@ -110,7 +113,7 @@ def clean_max_files_in_comment(cls, value: str) -> int: @classmethod def clean_coverage_path(cls, value: str) -> pathlib.Path: - return path_below(value) + return resolve_path(value) @classmethod def clean_annotations_output_path(cls, value: str) -> pathlib.Path: @@ -125,13 +128,15 @@ def clean_annotations_output_path(cls, value: str) -> pathlib.Path: def from_environ(cls, environ: MutableMapping[str, str]) -> Config: possible_variables = list(inspect.signature(cls).parameters) config_dict: dict[str, Any] = {k: v for k, v in environ.items() if k in possible_variables} - for key, value in list(config_dict.items()): - if hasattr(cls, f'clean_{key.lower()}'): - func: Callable = getattr(cls, f'clean_{key.lower()}') - try: - config_dict[key] = func(value) - except ValueError as exc: - raise ValueError(f'{key}: {exc!s}') from exc + for key, value in config_dict.items(): + if not hasattr(cls, f'clean_{key.lower()}'): + continue + + func: Callable = getattr(cls, f'clean_{key.lower()}') + try: + config_dict[key] = func(value) + except ValueError as exc: + raise ValueError(f'{key}: {exc!s}') from exc try: config_obj = cls(**config_dict) diff --git a/codecov/coverage/base.py b/codecov/coverage/base.py index fabcf6d..4040e2f 100644 --- a/codecov/coverage/base.py +++ b/codecov/coverage/base.py @@ -161,54 +161,55 @@ def parse_diff_output(self, diff: str) -> dict[pathlib.Path, list[int]]: if line.startswith(added_filename_prefix): current_file = pathlib.Path(line.removeprefix(added_filename_prefix)) continue - if line.startswith('@@'): - - def parse_line_number_diff_line(diff_line: str) -> Sequence[int]: - """ - Parse the "added" part of the line number diff text: - @@ -60,0 +61 @@ def compute_files( -> [64] - @@ -60,0 +61,9 @@ def compute_files( -> [64, 65, 66] - - Github API returns default context lines 3 at start and end, we need to remove them. - """ - start, _ = (int(i) for i in (diff_line.split()[2][1:] + ',1').split(',')[:2]) - - line_start = line_end = start - while diff_lines: - next_line = diff_lines.popleft() - if next_line.startswith(' '): - line_start += 1 - line_end += 1 - continue - - if next_line.startswith('-'): - continue - - diff_lines.appendleft(next_line) - break - - last_added_line = line_end - while diff_lines: - next_line = diff_lines.popleft() - if next_line.startswith(' ') or next_line.startswith('+'): - line_end += 1 - if next_line.startswith('+'): - last_added_line = line_end - continue - - if next_line.startswith('-'): - continue - - diff_lines.appendleft(next_line) - break - - return range(line_start, last_added_line) - - lines = parse_line_number_diff_line(diff_line=line) - if len(lines) > 0: - if current_file is None: - log.error('Diff output format is invalid: %s', diff) - raise ValueError - result.setdefault(current_file, []).extend(lines) + if not line.startswith('@@'): + continue + + def parse_line_number_diff_line(diff_line: str) -> Sequence[int]: + """ + Parse the "added" part of the line number diff text: + @@ -60,0 +61 @@ def compute_files( -> [64] + @@ -60,0 +61,9 @@ def compute_files( -> [64, 65, 66] + + Github API returns default context lines 3 at start and end, we need to remove them. + """ + start, _ = (int(i) for i in (diff_line.split()[2][1:] + ',1').split(',')[:2]) + + line_start = line_end = start + while diff_lines: + next_line = diff_lines.popleft() + if next_line.startswith(' '): + line_start += 1 + line_end += 1 + continue + + if next_line.startswith('-'): + continue + + diff_lines.appendleft(next_line) + break + + last_added_line = line_end + while diff_lines: + next_line = diff_lines.popleft() + if next_line.startswith(' ') or next_line.startswith('+'): + line_end += 1 + if next_line.startswith('+'): + last_added_line = line_end + continue + + if next_line.startswith('-'): + continue + + diff_lines.appendleft(next_line) + break + + return range(line_start, last_added_line) + + lines = parse_line_number_diff_line(diff_line=line) + if len(lines) > 0: + if current_file is None: + log.error('Diff output format is invalid: %s', diff) + raise ValueError + result.setdefault(current_file, []).extend(lines) return result diff --git a/codecov/github.py b/codecov/github.py index fc7031e..43031ca 100644 --- a/codecov/github.py +++ b/codecov/github.py @@ -61,57 +61,63 @@ def _init_user(self) -> User: ) raise CannotGetUser from exc - def _init_pr_number(self, pr_number: int | None = None, ref: str | None = None) -> tuple[int, str]: - if pr_number: - log.info('Getting pull request #%d.', pr_number) - try: - pull_request = self.client.repos(self.repository).pulls(pr_number).get() - if pull_request.state != 'open': - log.debug('Pull request #%d is not in open state.', pr_number) - raise NotFound + def _get_pr_details_from_pr_number(self, pr_number: int) -> tuple[int, str]: + log.info('Getting pull request #%d.', pr_number) + try: + pull_request = self.client.repos(self.repository).pulls(pr_number).get() + if pull_request.state != 'open': + log.debug('Pull request #%d is not in open state.', pr_number) + raise NotFound + + return pull_request.number, pull_request.head.ref + except Forbidden as exc: + log.error( + 'Forbidden access to pull request #%d. Insufficient permissions to retrieve details. Please verify the token permissions and try again.', + pr_number, + ) + + raise CannotGetPullRequest from exc + except NotFound as exc: + log.error( + 'Pull request #%d could not be found or is not in an open state. Please verify the pull request status.', + pr_number, + ) + + raise CannotGetPullRequest from exc - return pull_request.number, pull_request.head.ref - except Forbidden as exc: - log.error( - 'Forbidden access to pull request #%d. Insufficient permissions to retrieve details. Please verify the token permissions and try again.', - pr_number, - ) + def _get_pr_details_from_ref(self, ref: str) -> tuple[int, str]: + log.info('Getting pull request for branch %s.', ref) + try: + pull_requests = self.client.repos(self.repository).pulls.get(state='open', per_page=100) + for pull_request in pull_requests: + if pull_request.head.ref == ref: + return pull_request.number, pull_request.head.ref + log.debug( + 'No open pull request found for branch %s. Please ensure the branch has an active pull request.', + ref, + ) - raise CannotGetPullRequest from exc - except NotFound as exc: - log.error( - 'Pull request #%d could not be found or is not in an open state. Please verify the pull request status.', - pr_number, - ) + raise NotFound + except Forbidden as exc: + log.error( + 'Forbidden access to pull requests created for branch %s. Insufficient permissions to view pull request details.', + ref, + ) + raise CannotGetPullRequest from exc + except NotFound as exc: + log.error( + 'Checked the 100 most recent PRs in the repository, but no open pull request found for branch %s.', + ref, + ) + raise CannotGetPullRequest from exc - raise CannotGetPullRequest from exc + def _init_pr_number(self, pr_number: int | None = None, ref: str | None = None) -> tuple[int, str]: + if pr_number: + return self._get_pr_details_from_pr_number(pr_number) # If we're not on a PR, we need to find the PR number from the branch name if ref: - log.info('Getting pull request for branch %s.', ref) - try: - pull_requests = self.client.repos(self.repository).pulls.get(state='open', per_page=100) - for pull_request in pull_requests: - if pull_request.head.ref == ref: - return pull_request.number, pull_request.head.ref - log.debug( - 'No open pull request found for branch %s. Please ensure the branch has an active pull request.', - ref, - ) - - raise NotFound - except Forbidden as exc: - log.error( - 'Forbidden access to pull requests created for branch %s. Insufficient permissions to view pull request details.', - ref, - ) - raise CannotGetPullRequest from exc - except NotFound as exc: - log.error( - 'Checked the 100 most recent PRs in the repository, but no open pull request found for branch %s.', - ref, - ) - raise CannotGetPullRequest from exc + return self._get_pr_details_from_ref(ref) log.error('Pull request number or branch reference missing.') raise CannotGetPullRequest diff --git a/codecov/template.py b/codecov/template.py index bb3b711..77611cb 100644 --- a/codecov/template.py +++ b/codecov/template.py @@ -60,8 +60,7 @@ class FileInfo: diff: FileDiffCoverage | None -def get_comment_markdown( # pylint: disable=too-many-arguments,too-many-locals - *, +def get_comment_markdown( # pylint: disable=too-many-arguments,too-many-locals,too-many-positional-arguments coverage: Coverage, diff_coverage: DiffCoverage, files: list[FileInfo], diff --git a/tests/conftest.py b/tests/conftest.py index 517d759..a00be91 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -364,7 +364,7 @@ def request(self, method, path, **kwargs): match = False break else: - if not match_value == request_value: + if match_value != request_value: match = False break if match: diff --git a/tests/test_config.py b/tests/test_config.py index 919d713..23f52eb 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -14,26 +14,26 @@ def test_path_below_existing_file(): with tempfile.NamedTemporaryFile(suffix='.json') as temp_file: path = pathlib.Path(temp_file.name) - assert config.path_below(path) == path.resolve() + assert config.resolve_path(path) == path.resolve() def test_path_below_nonexistent_file(): path = pathlib.Path('/path/to/nonexistent_file.json') with pytest.raises(ValueError): - config.path_below(path) + config.resolve_path(path) def test_path_below_directory(): path = pathlib.Path('/path/to/directory') with pytest.raises(ValueError): - config.path_below(path) + config.resolve_path(path) def test_path_below_non_json_file(): with tempfile.NamedTemporaryFile(suffix='.txt') as temp_file: path = pathlib.Path(temp_file.name) with pytest.raises(ValueError): - config.path_below(path) + config.resolve_path(path) def test_config_from_environ_missing():