refactor: minor changes

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'.

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

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)

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

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

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],

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:

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