Skip to content

chore: fix log messages #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codecov/badge.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def get_badge_color(

def get_static_badge_url(label: str, message: str, color: str) -> str:
if not color or not message:
log.error('color and message are required')
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)
return 'https://img.shields.io/badge/' + urllib.parse.quote(f'{code}.svg')
4 changes: 4 additions & 0 deletions codecov/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ def clean_annotation_type(cls, value: str) -> AnnotationType:
def clean_github_pr_number(cls, value: str) -> int:
return int(value)

@classmethod
def clean_max_files_in_comment(cls, value: str) -> int:
return int(value)

@classmethod
def clean_coverage_path(cls, value: str) -> pathlib.Path:
return path_below(value)
Expand Down
8 changes: 1 addition & 7 deletions codecov/coverage/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,6 @@ class FileDiffCoverage:
# the statements (so it includes comments, blank lines, etc.)
added_lines: list[int]

# TODO: Remove this property and use `missing_statements` instead
# for backward compatibility
@property
def violation_lines(self) -> list[int]:
return self.missing_statements


@dataclasses.dataclass
class DiffCoverage:
Expand All @@ -91,7 +85,7 @@ def get_coverage_info(self, coverage_path: pathlib.Path) -> Coverage:
with coverage_path.open() as coverage_data:
json_coverage = json.loads(coverage_data.read())
except FileNotFoundError as exc:
log.error('Coverage report file not found: %s', coverage_path)
log.error('Coverage report file not found at the specified location: %s', coverage_path)
raise ConfigurationException from exc
except json.JSONDecodeError as exc:
log.error('Invalid JSON format in coverage report file: %s', coverage_path)
Expand Down
104 changes: 83 additions & 21 deletions codecov/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ def _init_user(self) -> User:
login=response.login,
)
except Unauthorized as exc:
log.error('Unauthorized access to user details. Invalid token.')
log.error('Authentication failed. The provided token is invalid. Please verify the token.')
raise CannotGetUser from exc
except Forbidden as exc:
log.error('Cannot get user details.')
log.error(
'Insufficient permissions. Unable to retrieve user details with the provided token. Please verify the token permissions and try again.',
)
raise CannotGetUser from exc

def _init_pr_number(self, pr_number: int | None = None, ref: str | None = None) -> tuple[int, str]:
Expand All @@ -70,10 +72,18 @@ def _init_pr_number(self, pr_number: int | None = None, ref: str | None = None)

return pull_request.number, pull_request.head.ref
except Forbidden as exc:
log.error('Forbidden access to pull request #%d.', pr_number)
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 does not exist or not in open state.', pr_number)
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

# If we're not on a PR, we need to find the PR number from the branch name
Expand All @@ -84,16 +94,26 @@ def _init_pr_number(self, pr_number: int | None = None, ref: str | None = None)
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.', 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 for branch %s.', ref)
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 recent updated 100 PRs, No open pull request found for branch %s.', ref)
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

log.error('No pull request number or branch reference provided.')
log.error('Pull request number or branch reference missing.')
raise CannotGetPullRequest

def _init_pr_diff(self) -> str:
Expand All @@ -105,18 +125,26 @@ def _init_pr_diff(self) -> str:
.get(use_text=True, headers={'Accept': 'application/vnd.github.v3.diff'})
)
except Forbidden as exc:
log.error('Forbidden access to pull request #%d diff.', self.pr_number)
log.error(
'Insufficient permissions to retrieve the diff of pull request #%d. Please verify the token permissions and try again.',
self.pr_number,
)
raise CannotGetPullRequest from exc
except NotFound as exc:
log.error('Pull request #%d does not exist or not in open state.', self.pr_number)
log.error(
'Pull request #%d does not exist or is not in an open state. Please ensure the branch has an active pull request.',
self.pr_number,
)
raise CannotGetPullRequest from exc

return pull_request_diff

def post_comment(self, contents: str, marker: str) -> None:
log.info('Posting comment on pull request #%d.', self.pr_number)
if len(contents) > 65536:
log.error('Comment exceeds allowed size(65536)')
log.error(
'Comment exceeds the 65536 character limit (GitHub limitation). Reduce the number of files to be reported in the comment using "MAX_FILES_IN_COMMENT" and try again.'
)
raise CannotPostComment

# Pull request review comments are comments made on a portion of the unified diff during a pull request review.
Expand All @@ -130,17 +158,26 @@ def post_comment(self, contents: str, marker: str) -> None:
comments_path(comment.id).patch(body=contents)
return
except Forbidden as exc:
log.error('Forbidden access to update comment.')
log.error(
'Insufficient permissions to update the comment on pull request #%d. Please verify the token permissions and try again.'
)
raise CannotPostComment from exc
except ApiError as exc:
log.error('Unknown Api error while updating comment.')
log.error(
'Error occurred while updating the comment on pull request #%d. Details: %s',
self.pr_number,
str(exc),
)
raise CannotPostComment from exc

log.info('Adding new comment on pull request')
try:
issue_comments_path.post(body=contents)
except Forbidden as exc:
log.error('Forbidden access to post comment.')
log.error(
'Insufficient permissions to post a comment on pull request #%d. Please check the token permissions and try again.',
self.pr_number,
)
raise CannotPostComment from exc

def write_annotations_to_branch(self, annotations: list[Annotation]) -> None:
Expand All @@ -155,10 +192,16 @@ def write_annotations_to_branch(self, annotations: list[Annotation]) -> None:
log.debug('Branch "%s/%s" is protected.', self.repository, self.annotations_data_branch)
raise NotFound
except Forbidden as exc:
log.error('Forbidden access to branch "%s/%s".', self.repository, self.annotations_data_branch)
log.error(
'Insufficient permissions to write annotations to the branch "%s/%s". Please verify the token permissions and ensure it has content read and write access.',
self.repository,
self.annotations_data_branch,
)
raise CannotGetBranch from exc
except NotFound as exc:
log.error('Branch "%s/%s" does not exist.', self.repository, self.annotations_data_branch)
log.error(
'Branch "%s/%s" either does not exist or is protected.', self.repository, self.annotations_data_branch
)
raise CannotGetBranch from exc

log.info('Writing annotations to branch.')
Expand All @@ -175,7 +218,11 @@ def write_annotations_to_branch(self, annotations: list[Annotation]) -> None:
self.annotations_data_branch,
)
except Forbidden as exc:
log.error('Forbidden access to branch "%s/%s".', self.repository, self.annotations_data_branch)
log.error(
'Insufficient permissions to write annotations to the branch "%s/%s". Please verify the token permissions and ensure it has content read and write access.',
self.repository,
self.annotations_data_branch,
)
raise CannotGetBranch from exc

try:
Expand All @@ -192,14 +239,29 @@ def write_annotations_to_branch(self, annotations: list[Annotation]) -> None:
content=encoded_content,
)
except NotFound as exc:
log.error('Branch "%s/%s" does not exist.', self.repository, self.annotations_data_branch)
log.error(
'Branch "%s/%s" either does not exist or is protected.', self.repository, self.annotations_data_branch
)
raise CannotGetBranch from exc
except Forbidden as exc:
log.error('Forbidden access to branch "%s/%s".', self.repository, self.annotations_data_branch)
log.error(
'Insufficient permissions to write annotations to the branch "%s/%s". Please verify the token permissions and ensure it has content read and write access.',
self.repository,
self.annotations_data_branch,
)
raise CannotGetBranch from exc
except Conflict as exc:
log.error('Conflict writing to branch "%s/%s".', self.repository, self.annotations_data_branch)
log.error(
'Conflict while adding #%s pull request annotation to branch "%s/%s".',
self.pr_number,
self.repository,
self.annotations_data_branch,
)
raise CannotGetBranch from exc
except ValidationFailed as exc:
log.error('Validation failed on committer name or email.')
log.error(
'Validation failed for committer name or email, or the endpoint was spammed while writing annotation to branch "%s/%s".',
self.repository,
self.annotations_data_branch,
)
raise CannotGetBranch from exc
4 changes: 3 additions & 1 deletion codecov/github_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ def _init_session(self) -> httpx.Client:
headers={'Authorization': f'token {self.token}'},
)
if not session:
log.error('GitHub client session could not be created. Invalid token configuration.')
log.error(
'Failed to create GitHub client session. This might be due to an internal configuration or code issue.'
)
raise ConfigurationException
return session

Expand Down
13 changes: 7 additions & 6 deletions codecov/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ def _init_log(self):

def _init_required(self):
if self.config.SKIP_COVERAGE and not self.config.ANNOTATE_MISSING_LINES:
log.error('Nothing to do since both SKIP_COVERAGE and ANNOTATE_MISSING_LINES are set to False. Exiting.')
log.error(
'No action taken as both SKIP_COVERAGE and ANNOTATE_MISSING_LINES are set to False. No comments or annotations will be generated.'
)
raise CoreProcessingException

def _init_github(self):
Expand All @@ -54,7 +56,7 @@ def _process_coverage(self):
try:
coverage = self.coverage_module.get_coverage_info(coverage_path=self.config.COVERAGE_PATH)
except ConfigurationException as e:
log.error('Error processing coverage data.')
log.error('Error parsing the coverage file. Please check the file and try again.')
raise CoreProcessingException from e

if self.config.BRANCH_COVERAGE:
Expand Down Expand Up @@ -105,15 +107,14 @@ def _process_pr(self):
)
except MissingMarker as e:
log.error(
'``{{ %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 "%s" not found. This marker is required to identify the comment and prevent creating or overwriting comments.',
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 for more information.'
'Rendering error occurred while generating the comment text for the PR. See the traceback for more details. Error: %s',
str(e),
)
raise CoreProcessingException from e

Expand Down
4 changes: 2 additions & 2 deletions codecov/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ def get_comment_markdown( # pylint: disable=too-many-arguments,too-many-locals
coverage_report_url=coverage_report_url,
)
except jinja2.exceptions.TemplateError as exc:
log.error('Error rendering template: %s', exc)
log.error('Template rendering error: %s', str(exc))
raise TemplateException from exc

if marker not in comment:
log.error('Marker not found in the comment template')
log.error('Marker is not found in the comment template.')
raise MissingMarker

return comment
Expand Down
14 changes: 0 additions & 14 deletions tests/coverage/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,6 @@ def extract_info(self, data):
return Coverage(meta=None, info=None, files={})


def test_diff_violations(make_coverage_and_diff):
_, diff = make_coverage_and_diff(
"""
# file: a.py
+ 1 line missing
2 line missing
+ 3 line missing
4 line covered
+ 5 line covered
"""
)
assert diff.files[pathlib.Path('a.py')].violation_lines == [1, 3]


class TestBase:
@pytest.mark.parametrize(
'num_covered, num_total, expected_coverage',
Expand Down