Skip to content
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.

Sign up for GitHub

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

fix github checks annotations #1052

Merged
merged 1 commit into from
Jan 31, 2025
Merged

fix github checks annotations #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions services/notification/notifiers/checks/patch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ def build_payload(
title = "Codecov Report"

checks_yaml_field = read_yaml_field(self.current_yaml, ("github_checks",))

should_annotate = (
checks_yaml_field.get("annotations", False)
if checks_yaml_field is not None
else True
)
try:
# checks_yaml_field can be dict, bool, None
# should_annotate defaults to False as of Jan 30 2025
should_annotate = checks_yaml_field.get("annotations", False)
except AttributeError:
should_annotate = False

flags = self.notifier_yaml_settings.get("flags")
paths = self.notifier_yaml_settings.get("paths")
Expand Down
101 changes: 73 additions & 28 deletions services/notification/notifiers/tests/unit/test_checks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,15 +762,7 @@ def test_build_default_payload(
"output": {
"title": "66.67% of diff hit (target 50.00%)",
"summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_default_payload/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n66.67% of diff hit (target 50.00%)",
"annotations": [
{
"path": "file_1.go",
"start_line": 10,
"end_line": 10,
"annotation_level": "warning",
"message": "Added line #L10 was not covered by tests",
}
],
"annotations": [],
},
"included_helper_text": {},
}
Expand Down Expand Up @@ -823,7 +815,7 @@ def test_build_payload_without_base_report(
title="default",
notifier_yaml_settings={},
notifier_site_settings=True,
current_yaml=UserYaml({}),
current_yaml=UserYaml({"github_checks": {"annotations": True}}),
repository_service=mock_repo_provider,
)
expected_result = {
Expand Down Expand Up @@ -869,15 +861,7 @@ def test_build_payload_target_coverage_failure_within_threshold(
"output": {
"title": "66.67% of diff hit (within 5.00% threshold of 70.00%)",
"summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_target_coverage_failure_within_threshold/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n66.67% of diff hit (within 5.00% threshold of 70.00%)",
"annotations": [
{
"path": "file_1.go",
"start_line": 10,
"end_line": 10,
"annotation_level": "warning",
"message": "Added line #L10 was not covered by tests",
}
],
"annotations": [],
},
"included_helper_text": {},
}
Expand Down Expand Up @@ -912,15 +896,7 @@ def test_build_payload_with_multiple_changes(
"output": {
"title": "50.00% of diff hit (target 76.92%)",
"summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_with_multiple_changes/{comparison_with_multiple_changes.head.commit.repository.name}/pull/{comparison_with_multiple_changes.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n50.00% of diff hit (target 76.92%)",
"annotations": [
{
"path": "modified.py",
"start_line": 23,
"end_line": 23,
"annotation_level": "warning",
"message": "Added line #L23 was not covered by tests",
}
],
"annotations": [],
},
"included_helper_text": {}, # not a custom target, no helper text
}
Expand All @@ -934,6 +910,75 @@ def test_build_payload_with_multiple_changes(
"segments"
) == multiple_diff_changes["files"][filename].get("segments")

def test_github_checks_annotations_yaml(
self,
comparison_with_multiple_changes,
mock_repo_provider,
mock_configuration,
multiple_diff_changes,
):
mock_repo_provider.get_compare.return_value = {"diff": multiple_diff_changes}
mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"

empty_annotations = []

# checks_yaml_field can be None
notifier = PatchChecksNotifier(
repository=comparison_with_multiple_changes.head.commit.repository,
title="default",
notifier_yaml_settings={},
notifier_site_settings=True,
current_yaml=UserYaml({}),
repository_service=mock_repo_provider,
)
result = notifier.build_payload(comparison_with_multiple_changes)
assert empty_annotations == result["output"]["annotations"]

# checks_yaml_field can be dict
notifier = PatchChecksNotifier(
repository=comparison_with_multiple_changes.head.commit.repository,
title="default",
notifier_yaml_settings={},
notifier_site_settings=True,
current_yaml=UserYaml({"github_checks": {"one": "two"}}),
repository_service=mock_repo_provider,
)
result = notifier.build_payload(comparison_with_multiple_changes)
assert empty_annotations == result["output"]["annotations"]

# checks_yaml_field can be bool
notifier = PatchChecksNotifier(
repository=comparison_with_multiple_changes.head.commit.repository,
title="default",
notifier_yaml_settings={},
notifier_site_settings=True,
current_yaml=UserYaml({"github_checks": False}),
repository_service=mock_repo_provider,
)
result = notifier.build_payload(comparison_with_multiple_changes)
assert empty_annotations == result["output"]["annotations"]

# checks_yaml_field with annotations
notifier = PatchChecksNotifier(
repository=comparison_with_multiple_changes.head.commit.repository,
title="default",
notifier_yaml_settings={},
notifier_site_settings=True,
current_yaml=UserYaml({"github_checks": {"annotations": True}}),
repository_service=mock_repo_provider,
)
expected_annotations = [
{
"path": "modified.py",
"start_line": 23,
"end_line": 23,
"annotation_level": "warning",
"message": "Added line #L23 was not covered by tests",
}
]
result = notifier.build_payload(comparison_with_multiple_changes)
assert expected_annotations == result["output"]["annotations"]

def test_build_payload_no_diff(
self, sample_comparison, mock_repo_provider, mock_configuration
):
Expand Down
Loading