diff --git a/README.md b/README.md index 5178e20..66a5d2f 100644 --- a/README.md +++ b/README.md @@ -90,9 +90,9 @@ Note: Either `GITHUB_PR_NUMBER` or `GITHUB_REF` is required. `GITHUB_PR_NUMBER` - `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_OUTPUT_PATH`: The path where the annotaions should be stored. Should be a .json file. +- `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. Default is 'warning'. +- `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. - `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. diff --git a/codecov/config.py b/codecov/config.py index 49b9dc8..52fc5b3 100644 --- a/codecov/config.py +++ b/codecov/config.py @@ -5,11 +5,10 @@ import inspect import pathlib from collections.abc import MutableMapping +from enum import Enum from typing import Any, Callable -from codecov.exceptions import InvalidAnnotationType, MissingEnvironmentVariable - -# TODO: Rename this file to config.py +from codecov.exceptions import MissingEnvironmentVariable def path_below(path_str: str | pathlib.Path) -> pathlib.Path: @@ -26,8 +25,14 @@ def str_to_bool(value: str) -> bool: return value.lower() in ('1', 'true', 'yes') +class AnnotationType(Enum): + NOTICE = 'notice' + WARNING = 'warning' + ERROR = 'error' + + # pylint: disable=invalid-name, too-many-instance-attributes -@dataclasses.dataclass +@dataclasses.dataclass(kw_only=True) class Config: """This object defines the environment variables""" @@ -44,8 +49,7 @@ class Config: BRANCH_COVERAGE: bool = False SKIP_COVERAGE: bool = False ANNOTATE_MISSING_LINES: bool = False - # TODO: Make it enum - ANNOTATION_TYPE: str = 'warning' + ANNOTATION_TYPE: AnnotationType = AnnotationType.WARNING ANNOTATIONS_OUTPUT_PATH: pathlib.Path | None = None ANNOTATIONS_DATA_BRANCH: str | None = None MAX_FILES_IN_COMMENT: int = 25 @@ -88,12 +92,8 @@ def clean_debug(cls, value: str) -> bool: return str_to_bool(value) @classmethod - def clean_annotation_type(cls, value: str) -> str: - if value not in {'notice', 'warning', 'error'}: - raise InvalidAnnotationType( - f'The annotation type {value} is not valid. Please choose from notice, warning or error' - ) - return value + def clean_annotation_type(cls, value: str) -> AnnotationType: + return AnnotationType(value) @classmethod def clean_github_pr_number(cls, value: str) -> int: @@ -105,7 +105,10 @@ def clean_coverage_path(cls, value: str) -> pathlib.Path: @classmethod def clean_annotations_output_path(cls, value: str) -> pathlib.Path: - return pathlib.Path(value) + path = pathlib.Path(value) + if path.exists() or path.is_dir(): + return path + raise ValueError # We need to type environ as a MutableMapping because that's what # os.environ is, and `dict[str, str]` is not enough diff --git a/codecov/coverage/base.py b/codecov/coverage/base.py index 5ffb9df..15fda5e 100644 --- a/codecov/coverage/base.py +++ b/codecov/coverage/base.py @@ -9,6 +9,7 @@ from collections import deque from collections.abc import Sequence +from codecov.exceptions import ConfigurationException from codecov.log import log @@ -86,16 +87,15 @@ def compute_coverage(self, num_covered: int, num_total: int) -> decimal.Decimal: return decimal.Decimal(num_covered) / decimal.Decimal(num_total) def get_coverage_info(self, coverage_path: pathlib.Path) -> Coverage: - # TODO: Write a custom exception here and handle it in the main script try: with coverage_path.open() as coverage_data: json_coverage = json.loads(coverage_data.read()) - except FileNotFoundError: + except FileNotFoundError as exc: log.error('Coverage report file not found: %s', coverage_path) - raise - except json.JSONDecodeError: + raise ConfigurationException from exc + except json.JSONDecodeError as exc: log.error('Invalid JSON format in coverage report file: %s', coverage_path) - raise + raise ConfigurationException from exc return self.extract_info(data=json_coverage) diff --git a/codecov/exceptions.py b/codecov/exceptions.py index f12c556..56bfbbb 100644 --- a/codecov/exceptions.py +++ b/codecov/exceptions.py @@ -50,10 +50,6 @@ class MissingEnvironmentVariable(ConfigurationException): pass -class InvalidAnnotationType(ConfigurationException): - pass - - class TemplateBaseException(CoreBaseException): pass diff --git a/codecov/github.py b/codecov/github.py index 90bf26f..66e6da9 100644 --- a/codecov/github.py +++ b/codecov/github.py @@ -120,9 +120,8 @@ def post_comment(self, contents: str, marker: str) -> None: log.error('Comment exceeds allowed size(65536)') raise CannotPostComment - # TODO: Try and see if you can refactor this to use single request - # issue_comments_path.patch(body=contents) - # TODO: Make scope only for pull request not for issues + # Pull request review comments are comments made on a portion of the unified diff during a pull request review. + # Issue comments are comments on the entire pull request. We need issue comments. issue_comments_path = self.client.repos(self.repository).issues(self.pr_number).comments comments_path = self.client.repos(self.repository).issues.comments for comment in issue_comments_path.get(): diff --git a/codecov/github_client.py b/codecov/github_client.py index 20a534e..edb5535 100644 --- a/codecov/github_client.py +++ b/codecov/github_client.py @@ -1,5 +1,7 @@ from __future__ import annotations +from typing import Any + import httpx from codecov.exceptions import ApiError, ConfigurationException, Conflict, Forbidden, NotFound, ValidationFailed @@ -80,7 +82,7 @@ def __getattr__(self, attr): def _http(self, method: str, path: str, *, use_bytes: bool = False, use_text: bool = False, **kw): _method = method.lower() - requests_kwargs = {} + requests_kwargs: dict[Any, Any] = {} headers = kw.pop('headers', {}) if _method == 'get' and kw: requests_kwargs = {'params': kw} @@ -106,14 +108,16 @@ def _http(self, method: str, path: str, *, use_bytes: bool = False, use_text: bo try: response.raise_for_status() except httpx.HTTPStatusError as exc: - # TODO: Use `match` Statement - cls: type[ApiError] = { - 403: Forbidden, - 404: NotFound, - 409: Conflict, - 422: ValidationFailed, - }.get(exc.response.status_code, ApiError) - - raise cls(str(contents)) from exc + exc_cls = ApiError + match exc.response.status_code: + case 403: + exc_cls = Forbidden + case 404: + exc_cls = NotFound + case 409: + exc_cls = Conflict + case 422: + exc_cls = ValidationFailed + raise exc_cls(str(contents)) from exc return contents diff --git a/codecov/main.py b/codecov/main.py index 0e3b8e6..1dccee7 100644 --- a/codecov/main.py +++ b/codecov/main.py @@ -5,7 +5,7 @@ from codecov.config import Config from codecov.coverage import PytestCoverage from codecov.coverage.base import Coverage, DiffCoverage -from codecov.exceptions import CoreProcessingException, MissingMarker, TemplateException +from codecov.exceptions import ConfigurationException, CoreProcessingException, MissingMarker, TemplateException from codecov.github import Github from codecov.github_client import GitHubClient from codecov.log import log, setup as log_setup @@ -51,7 +51,12 @@ def run(self): def _process_coverage(self): log.info('Processing coverage data') - coverage = self.coverage_module.get_coverage_info(coverage_path=self.config.COVERAGE_PATH) + try: + coverage = self.coverage_module.get_coverage_info(coverage_path=self.config.COVERAGE_PATH) + except ConfigurationException as e: + log.error('Error processing coverage data.') + raise CoreProcessingException from e + if self.config.BRANCH_COVERAGE: coverage = diff_grouper.fill_branch_missing_groups(coverage=coverage) added_lines = self.coverage_module.parse_diff_output(diff=self.github.pr_diff) @@ -64,7 +69,7 @@ def _process_pr(self): log.info('Skipping coverage report generation.') return - log.info('Generating comment for PR') + 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( coverage=self.coverage, @@ -99,18 +104,15 @@ def _process_pr(self): ) except MissingMarker as e: log.error( - 'Marker not found. This error can happen if you defined a custom comment ' - "template that doesn't inherit the base template and you didn't include " - '``{{ marker }}``. The marker is necessary for this action to recognize ' - "its own comment and avoid making new comments or overwriting someone else's " - 'comment.' + '``{{ %s }}`` marker not found. The marker is necessary for this action to recognize ' + "its own comment and avoid making new comments or overwriting someone else's comment.", + marker, ) raise CoreProcessingException from e except TemplateException as e: log.error( 'There was a rendering error when computing the text of the comment to post ' - "on the PR. Please see the traceback, in particular if you're using a custom " - 'template.' + 'on the PR. Please see the traceback for more information.' ) raise CoreProcessingException from e @@ -125,7 +127,7 @@ def _generate_annotations(self): log.info('Generating annotations for missing lines.') annotations = diff_grouper.get_diff_missing_groups(coverage=self.coverage, diff_coverage=self.diff_coverage) formatted_annotations = groups.create_missing_coverage_annotations( - annotation_type=self.config.ANNOTATION_TYPE, + annotation_type=self.config.ANNOTATION_TYPE.value, annotations=annotations, ) @@ -136,7 +138,7 @@ def _generate_annotations(self): ) formatted_annotations.extend( groups.create_missing_coverage_annotations( - annotation_type=self.config.ANNOTATION_TYPE, + annotation_type=self.config.ANNOTATION_TYPE.value, annotations=branch_annotations, branch=True, ) @@ -155,12 +157,13 @@ def _generate_annotations(self): print(reset, end='') # Save to file - # TODO: Take the folder path instead of the file path + file_name = f'{self.github.pr_number}-annotations.json' if self.config.ANNOTATIONS_OUTPUT_PATH: - log.info('Writing annotations to file.') - with self.config.ANNOTATIONS_OUTPUT_PATH.open('w+') as annotations_file: + log.info('Writing annotations to file %s', file_name) + with self.config.ANNOTATIONS_OUTPUT_PATH.joinpath(file_name).open('w+') as annotations_file: json.dump(formatted_annotations, annotations_file, cls=groups.AnnotationEncoder) + # Write to branch if self.config.ANNOTATIONS_DATA_BRANCH: log.info('Writing annotations to branch.') self.github.write_annotations_to_branch(annotations=formatted_annotations) diff --git a/docs/annotations.md b/docs/annotations.md index 1312448..d7d6ba0 100644 --- a/docs/annotations.md +++ b/docs/annotations.md @@ -8,8 +8,8 @@ By default, these annotations are written to the console, but you can also choos ## Storing Annotations -1. **To a File**: - - Set the file path in `ANNOTATIONS_OUTPUT_PATH`. +1. **To a Folder**: + - Set the folder path in `ANNOTATIONS_OUTPUT_PATH`. 2. **To a Branch**: - Set the branch name in `ANNOTATIONS_DATA_BRANCH`. diff --git a/tests/conftest.py b/tests/conftest.py index f65afd8..517d759 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -92,14 +92,14 @@ def _(code: str, has_branches: bool = True) -> Coverage: ) if any( x in line - for x in { + for x in ( 'line covered', 'line missing', 'line excluded', 'branch covered', 'branch missing', 'branch partial', - } + ) ): coverage_obj.files[current_file].info.num_statements += 1 coverage_obj.info.num_statements += 1 diff --git a/tests/coverage/test_base.py b/tests/coverage/test_base.py index 049e377..53adb2a 100644 --- a/tests/coverage/test_base.py +++ b/tests/coverage/test_base.py @@ -10,6 +10,7 @@ from codecov.coverage import Coverage, DiffCoverage, FileDiffCoverage from codecov.coverage.base import BaseCoverage +from codecov.exceptions import ConfigurationException class BaseCoverageDemo(BaseCoverage): @@ -54,10 +55,10 @@ def test_get_coverage_info(self, coverage_json): with patch('pathlib.Path.open') as mock_open: mock_open.return_value.__enter__.return_value.read.return_value = b'invalid json' - with pytest.raises(json.JSONDecodeError): + with pytest.raises(ConfigurationException): BaseCoverageDemo().get_coverage_info(pathlib.Path(tempfile.mkstemp(suffix='.json')[1])) - with pytest.raises(FileNotFoundError): + with pytest.raises(ConfigurationException): BaseCoverageDemo().get_coverage_info(pathlib.Path('path/to/file.json')) @pytest.mark.parametrize( diff --git a/tests/test_config.py b/tests/test_config.py index 39ffb1f..896ea82 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -8,7 +8,7 @@ import pytest from codecov import config -from codecov.exceptions import InvalidAnnotationType, MissingEnvironmentVariable +from codecov.exceptions import MissingEnvironmentVariable def test_path_below_existing_file(): @@ -41,46 +41,47 @@ def test_config_from_environ_missing(): config.Config.from_environ({}) -def test_config__from_environ__sample(): +def test_config_from_environ_sample(): token = secrets.token_urlsafe() with tempfile.NamedTemporaryFile(suffix='.json') as temp_file: - assert config.Config.from_environ( - { - 'GITHUB_REPOSITORY': 'your_repository', - 'COVERAGE_PATH': temp_file.name, - 'GITHUB_TOKEN': token, - 'GITHUB_PR_NUMBER': '123', - 'GITHUB_REF': 'main', - 'SUBPROJECT_ID': 'your_subproject_id', - 'MINIMUM_GREEN': '90', - 'MINIMUM_ORANGE': '70', - 'SKIP_COVERAGE': 'False', - 'ANNOTATE_MISSING_LINES': 'True', - 'ANNOTATION_TYPE': 'warning', - 'ANNOTATIONS_OUTPUT_PATH': '/path/to/annotations', - 'MAX_FILES_IN_COMMENT': 25, - 'COMPLETE_PROJECT_REPORT': 'True', - 'COVERAGE_REPORT_URL': 'https://your_coverage_report_url', - 'DEBUG': 'False', - } - ) == config.Config( - GITHUB_REPOSITORY='your_repository', - COVERAGE_PATH=pathlib.Path(temp_file.name).resolve(), - GITHUB_TOKEN=token, # noqa: S106 - GITHUB_PR_NUMBER=123, - GITHUB_REF='main', - SUBPROJECT_ID='your_subproject_id', - MINIMUM_GREEN=decimal.Decimal('90'), - MINIMUM_ORANGE=decimal.Decimal('70'), - SKIP_COVERAGE=False, - ANNOTATE_MISSING_LINES=True, - ANNOTATION_TYPE='warning', - ANNOTATIONS_OUTPUT_PATH=pathlib.Path('/path/to/annotations'), - MAX_FILES_IN_COMMENT=25, - COMPLETE_PROJECT_REPORT=True, - COVERAGE_REPORT_URL='https://your_coverage_report_url', - DEBUG=False, - ) + with tempfile.TemporaryDirectory() as tmp_dir: + assert config.Config.from_environ( + { + 'GITHUB_REPOSITORY': 'your_repository', + 'COVERAGE_PATH': temp_file.name, + 'GITHUB_TOKEN': token, + 'GITHUB_PR_NUMBER': '123', + 'GITHUB_REF': 'main', + 'SUBPROJECT_ID': 'your_subproject_id', + 'MINIMUM_GREEN': '90', + 'MINIMUM_ORANGE': '70', + 'SKIP_COVERAGE': 'False', + 'ANNOTATE_MISSING_LINES': 'True', + 'ANNOTATION_TYPE': 'notice', + 'ANNOTATIONS_OUTPUT_PATH': str(tmp_dir), + 'MAX_FILES_IN_COMMENT': 25, + 'COMPLETE_PROJECT_REPORT': 'True', + 'COVERAGE_REPORT_URL': 'https://your_coverage_report_url', + 'DEBUG': 'False', + } + ) == config.Config( + GITHUB_REPOSITORY='your_repository', + COVERAGE_PATH=pathlib.Path(temp_file.name).resolve(), + GITHUB_TOKEN=token, # noqa: S106 + GITHUB_PR_NUMBER=123, + GITHUB_REF='main', + SUBPROJECT_ID='your_subproject_id', + MINIMUM_GREEN=decimal.Decimal('90'), + MINIMUM_ORANGE=decimal.Decimal('70'), + SKIP_COVERAGE=False, + ANNOTATE_MISSING_LINES=True, + ANNOTATION_TYPE=config.AnnotationType.NOTICE, + ANNOTATIONS_OUTPUT_PATH=pathlib.Path(tmp_dir), + MAX_FILES_IN_COMMENT=25, + COMPLETE_PROJECT_REPORT=True, + COVERAGE_REPORT_URL='https://your_coverage_report_url', + DEBUG=False, + ) def test_config_required_pr_or_ref(): @@ -96,7 +97,7 @@ def test_config_required_pr_or_ref(): def test_config_invalid_annotation_type(): - with pytest.raises(InvalidAnnotationType): + with pytest.raises(ValueError): config.Config.from_environ({'ANNOTATION_TYPE': 'foo'}) @@ -155,11 +156,11 @@ def test_config_clean_debug(): def test_config_clean_annotation_type(): value = config.Config.clean_annotation_type('warning') - assert value == 'warning' + assert value == config.AnnotationType.WARNING def test_config_clean_annotation_type_invalid(): - with pytest.raises(InvalidAnnotationType): + with pytest.raises(ValueError): config.Config.clean_annotation_type('foo') @@ -175,8 +176,12 @@ def test_config_clean_coverage_path(): def test_config_clean_annotations_output_path(): - value = config.Config.clean_annotations_output_path('/path/to/annotations') - assert value == pathlib.Path('/path/to/annotations') + with tempfile.TemporaryDirectory() as temp_dir: + value = config.Config.clean_annotations_output_path(temp_dir) + assert value == pathlib.Path(temp_dir) + + with pytest.raises(ValueError): + config.Config.clean_annotations_output_path('/path/to/nonexistent_dir') def test_str_to_bool_invalid(): diff --git a/tests/test_main.py b/tests/test_main.py index 27f1c5a..8459e73 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -5,7 +5,7 @@ import pytest -from codecov.exceptions import CoreProcessingException, MissingMarker, TemplateException +from codecov.exceptions import ConfigurationException, CoreProcessingException, MissingMarker, TemplateException from codecov.main import Main @@ -27,6 +27,14 @@ def test_init(self, test_config, gh): assert main.github.annotations_data_branch == test_config.ANNOTATIONS_DATA_BRANCH def test_process_coverage(self, test_config, gh, coverage_obj, diff_coverage_obj): + with patch.object(Main, '_init_config', return_value=test_config): + with patch.object(Main, '_init_github', return_value=gh): + main = Main() + main.coverage_module = MagicMock() + main.coverage_module.get_coverage_info = MagicMock(side_effect=ConfigurationException) + with pytest.raises(CoreProcessingException): + main._process_coverage() + with patch.object(Main, '_init_config', return_value=test_config): with patch.object(Main, '_init_github', return_value=gh): main = Main() @@ -158,7 +166,7 @@ def test_generate_annotations_write_to_file( with patch.object(Main, '_init_github', return_value=gh): main = Main() main.config.ANNOTATE_MISSING_LINES = True - main.config.ANNOTATIONS_OUTPUT_PATH = pathlib.Path(tempfile.mkstemp(suffix='.json')[1]) + main.config.ANNOTATIONS_OUTPUT_PATH = pathlib.Path(tempfile.mkdtemp()) main.coverage = coverage_obj main.diff_coverage = diff_coverage_obj assert main._generate_annotations() is None @@ -170,7 +178,7 @@ def test_generate_annotations_write_to_file( main = Main() main.config.BRANCH_COVERAGE = True main.config.ANNOTATE_MISSING_LINES = True - main.config.ANNOTATIONS_OUTPUT_PATH = pathlib.Path(tempfile.mkstemp(suffix='.json')[1]) + main.config.ANNOTATIONS_OUTPUT_PATH = pathlib.Path(tempfile.mkdtemp()) main.coverage = coverage_obj main.diff_coverage = diff_coverage_obj assert main._generate_annotations() is None