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.

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

making changes to the new header section based on the designs #104

Merged
merged 4 commits into from
Sep 22, 2023
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
124 changes: 35 additions & 89 deletions services/notification/notifiers/mixins/message/sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,107 +109,53 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non
pull_dict = comparison.enriched_pull.provider_pull
repo_service = comparison.repository_service.service

change = (
Decimal(head_report.totals.coverage) - Decimal(base_report.totals.coverage)
if base_report and head_report
else Decimal(0)
)
rounded_change = Decimal(round_number(yaml, change))
diff_totals = head_report.apply_diff(diff)
if diff_totals:
patch_cov = (
round_number(yaml, Decimal(diff_totals.coverage))
if diff_totals.coverage
else None
)
misses_and_partials = diff_totals.misses + diff_totals.partials
else:
patch_cov = None
misses_and_partials = None

hide_project_coverage = self.settings.get("hide_project_coverage", False)
if hide_project_coverage:
if misses_and_partials:
yield (
f"Attention: `{misses_and_partials} lines` in your changes are missing coverage. Please review."
)
else:
yield "All modified lines are covered by tests :white_check_mark:"
if misses_and_partials:
yield (
f"Attention: `{misses_and_partials} lines` in your changes are missing coverage. Please review."
)
else:
if base_report and head_report:
patch_cov_msg = (
f"Patch coverage: **`{patch_cov}%`**"
if patch_cov
else "Patch coverage has no change"
)
yield "All modified lines are covered by tests :white_check_mark:"

if rounded_change > 0:
project_cov_change_msg = (
f"project coverage change: **`+{rounded_change}%`** :tada:"
)
elif rounded_change < 0:
project_cov_change_msg = (
f"project coverage change: **`{rounded_change}%`** :warning:"
)
else:
project_cov_change_msg = "no project coverage change."

if not patch_cov and rounded_change == 0:
yield (f"Patch and project coverage have no change.")
else:
yield (f"{patch_cov_msg} and {project_cov_change_msg}")
hide_project_coverage = self.settings.get("hide_project_coverage", False)
if hide_project_coverage:
return

yield (
"> Comparison is base [(`{commitid_base}`)]({links[base]}?el=desc) {base_cov}% compared to head [(`{commitid_head}`)]({links[pull]}?src=pr&el=desc) {head_cov}%.".format(
pull=pull.pullid,
base=pull_dict["base"]["branch"],
commitid_head=comparison.head.commit.commitid[:7],
commitid_base=comparison.base.commit.commitid[:7],
links=links,
base_cov=round_number(
yaml, Decimal(base_report.totals.coverage)
),
head_cov=round_number(
yaml, Decimal(head_report.totals.coverage)
),
)
if base_report and head_report:
yield (
"> Comparison is base [(`{commitid_base}`)]({links[base]}?el=desc) {base_cov}% compared to head [(`{commitid_head}`)]({links[pull]}?src=pr&el=desc) {head_cov}%.".format(
pull=pull.pullid,
base=pull_dict["base"]["branch"],
commitid_head=comparison.head.commit.commitid[:7],
commitid_base=comparison.base.commit.commitid[:7],
links=links,
base_cov=round_number(yaml, Decimal(base_report.totals.coverage)),
head_cov=round_number(yaml, Decimal(head_report.totals.coverage)),
)

else:
yield (
"> :exclamation: No coverage uploaded for {request_type} {what} (`{branch}@{commit}`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-{what}-commit).".format(
what="base" if not base_report else "head",
branch=pull_dict["base" if not base_report else "head"][
"branch"
],
commit=pull_dict["base" if not base_report else "head"][
"commitid"
][:7],
request_type="merge request"
if repo_service == "gitlab"
else "pull request",
)
)
else:
yield (
"> :exclamation: No coverage uploaded for {request_type} {what} (`{branch}@{commit}`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-{what}-commit).".format(
what="base" if not base_report else "head",
branch=pull_dict["base" if not base_report else "head"]["branch"],
commit=pull_dict["base" if not base_report else "head"]["commitid"][
:7
],
request_type="merge request"
if repo_service == "gitlab"
else "pull request",
)
)

diff_totals = head_report.apply_diff(diff)
if diff_totals and diff_totals.coverage is not None:
yield (
"> Patch coverage: {percentage}% of modified lines in {request_type} are covered.".format(
percentage=round_number(
yaml, Decimal(diff_totals.coverage)
),
request_type="merge request"
if repo_service == "gitlab"
else "pull request",
)
)
else:
yield "> Patch has no changes to coverable lines."

if behind_by:
yield (
f"> Report is {behind_by} commits behind head on {pull_dict['base']['branch']}."
)
if behind_by:
yield (
f"> Report is {behind_by} commits behind head on {pull_dict['base']['branch']}."
)
if (
comparison.enriched_pull.provider_pull is not None
and comparison.head.commit.commitid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ async def test_notify_new_layout(
assert result.explanation is None
message = [
"## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=h1) Report",
"Patch coverage has no change and project coverage change: **`+10.00%`** :tada:",
"All modified lines are covered by tests :white_check_mark:",
"> Comparison is base [(`5b174c2`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) 50.00% compared to head [(`5601846`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc) 60.00%.",
"> Report is 2 commits behind head on main.",
"",
Expand Down Expand Up @@ -638,7 +638,7 @@ async def test_notify_with_components(
assert result.explanation is None
message = [
"## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=h1) Report",
"Patch coverage has no change and project coverage change: **`+10.00%`** :tada:",
"All modified lines are covered by tests :white_check_mark:",
"> Comparison is base [(`5b174c2`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) 50.00% compared to head [(`5601846`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc) 60.00%.",
"> Report is 2 commits behind head on main.",
"",
Expand Down
20 changes: 9 additions & 11 deletions services/notification/notifiers/tests/unit/test_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -1743,15 +1743,13 @@ async def test_build_message_negative_change_tricky_rounding_newheader(
result = await notifier.build_message(comparison)
expected_result = [
f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report",
f"Patch coverage has no change and project coverage change: **`-0.04%`** :warning:",
f"All modified lines are covered by tests :white_check_mark:",
f"> Comparison is base [(`{comparison.base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.base.commit.commitid}?el=desc) 88.58% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 88.54%.",
f"",
]
print(result)
li = 0
for exp, res in zip(expected_result, result):
li += 1
print(li)
assert exp == res
assert result == expected_result

Expand Down Expand Up @@ -3905,7 +3903,7 @@ async def test_new_header_section_writer(self, mocker, sample_comparison):
)
print(res)
assert res == [
"Patch and project coverage have no change.",
"All modified lines are covered by tests :white_check_mark:",
f"> Comparison is base [(`{sample_comparison.base.commit.commitid[:7]}`)](urlurl?el=desc) 0% compared to head [(`{sample_comparison.head.commit.commitid[:7]}`)](urlurl?src=pr&el=desc) 0%.",
]

Expand Down Expand Up @@ -3935,7 +3933,7 @@ async def test_new_header_section_writer_with_behind_by(
)
print(res)
assert res == [
"Patch and project coverage have no change.",
"All modified lines are covered by tests :white_check_mark:",
f"> Comparison is base [(`{sample_comparison.base.commit.commitid[:7]}`)](urlurl?el=desc) 0% compared to head [(`{sample_comparison.head.commit.commitid[:7]}`)](urlurl?src=pr&el=desc) 0%.",
"> Report is 3 commits behind head on master.",
]
Expand Down Expand Up @@ -4224,7 +4222,7 @@ async def test_create_message_files_section_with_critical_files_new_layout(
repository = sample_comparison.head.commit.repository
expected_result = [
f"## [Codecov](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report",
f"Patch coverage has no change and project coverage change: **`+10.00%`** :tada:",
f"All modified lines are covered by tests :white_check_mark:",
f"> Comparison is base [(`{comparison.base.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/commit/{comparison.base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.",
"",
"Changes have been made to critical files, which contain lines commonly executed in production. [Learn more](https://docs.codecov.com/docs/impact-analysis)",
Expand Down Expand Up @@ -4268,8 +4266,8 @@ async def test_build_message_no_base_commit_new_layout(
result = await notifier.build_message(comparison)
expected_result = [
f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report",
f"All modified lines are covered by tests :white_check_mark:",
f"> :exclamation: No coverage uploaded for pull request base (`master@cdf9aa4`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).",
f"> Patch has no changes to coverable lines.",
f"",
f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)",
f"",
Expand Down Expand Up @@ -4332,8 +4330,8 @@ async def test_build_message_no_base_report_new_layout(
result = await notifier.build_message(comparison)
expected_result = [
f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report",
f"Attention: `1 lines` in your changes are missing coverage. Please review.",
f"> :exclamation: No coverage uploaded for pull request base (`master@{comparison.base.commit.commitid[:7]}`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).",
f"> Patch coverage: 66.67% of modified lines in pull request are covered.",
f"",
f"<details><summary>Additional details and impacted files</summary>\n",
f"",
Expand Down Expand Up @@ -4442,7 +4440,7 @@ async def test_build_message_head_and_pull_head_differ_new_layout(
result = await notifier.build_message(comparison)
expected_result = [
f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report",
f"Patch coverage: **`66.67%`** and project coverage change: **`+10.00%`** :tada:",
f"Attention: `1 lines` in your changes are missing coverage. Please review.",
f"> Comparison is base [(`{comparison.base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.",
f"",
f"> :exclamation: Current head {comparison.head.commit.commitid[:7]} differs from pull request most recent head {comparison.enriched_pull.provider_pull['head']['commitid'][:7]}. Consider uploading reports for the commit {comparison.enriched_pull.provider_pull['head']['commitid'][:7]} to get more accurate results",
Expand Down Expand Up @@ -4517,7 +4515,7 @@ async def test_build_message_head_and_pull_head_differ_with_components(
result = await notifier.build_message(comparison)
expected_result = [
f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report",
f"Patch coverage: **`66.67%`** and project coverage change: **`+10.00%`** :tada:",
f"Attention: `1 lines` in your changes are missing coverage. Please review.",
f"> Comparison is base [(`{comparison.base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.",
f"",
f"> :exclamation: Current head {comparison.head.commit.commitid[:7]} differs from pull request most recent head {comparison.enriched_pull.provider_pull['head']['commitid'][:7]}. Consider uploading reports for the commit {comparison.enriched_pull.provider_pull['head']['commitid'][:7]} to get more accurate results",
Expand Down Expand Up @@ -4588,7 +4586,7 @@ async def test_build_message_no_patch_or_proj_change(
result = await notifier.build_message(comparison)
expected_result = [
f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report",
f"Patch and project coverage have no change.",
f"All modified lines are covered by tests :white_check_mark:",
f"> Comparison is base [(`{comparison.base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.base.commit.commitid}?el=desc) 65.38% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 65.38%.",
f"",
f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)",
Expand Down