Skip to content

Commit

Permalink
fix: reorganize TA finishing and error commenting
Browse files Browse the repository at this point in the history 
this commit makes it so there is a single flow for TA finishing where
we will always attempt to comment if there is relevant data to comment
and we will only persist data if there is new data (uploads) to persist

in the new test results flow the error comment will no longer be used
and there will be a single notify function that has the capability of
choosing which section of the comment to show: if there are errors show
them, if there are relevant failures show them
  • Loading branch information
@joseph-sentry
joseph-sentry committed Dec 27, 2024
1 parent 9185aef commit c650bbe
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 123 deletions.
108 changes: 58 additions & 50 deletions services/test_results.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,18 @@ class FlakeInfo:
count: int


@dataclass
class TACommentInDepthInfo:
failures: list[TestResultsNotificationFailure]
flaky_tests: dict[str, FlakeInfo]


@dataclass
class TestResultsNotificationPayload:
failed: int
passed: int
skipped: int
failures: list[TestResultsNotificationFailure]
flaky_tests: dict[str, FlakeInfo]
info: TACommentInDepthInfo | None = None


def wrap_in_details(summary: str, content: str):
Expand Down Expand Up @@ -237,12 +242,12 @@ def messagify_flake(
return make_quoted(f"{test_name}\n{flake_rate_section}\n{stack_trace}")


def specific_error_message(upload_errors: list[UploadError]) -> str:
title = f"### :x: {upload_errors[0].error_code}"
def specific_error_message(upload_error: UploadError) -> str:
title = f"### :x: {upload_error.error_code}"
description = "\n".join(
[
f"Upload processing failed due to {upload_errors[0].error_code.lower()}. Please review the parser error message:",
f"`{upload_errors[0].error_params['error_message']}`",
f"Upload processing failed due to {upload_error.error_code.lower()}. Please review the parser error message:",
f"`{upload_error.error_params['error_message']}`",
"For more help, visit our [troubleshooting guide](https://docs.codecov.com/docs/test-analytics#troubleshooting).",
]
)
Expand All @@ -256,65 +261,70 @@ def specific_error_message(upload_errors: list[UploadError]) -> str:
@dataclass
class TestResultsNotifier(BaseNotifier):
payload: TestResultsNotificationPayload | None = None
errors: list[UploadError] | None = None
error: UploadError | None = None

def build_message(self) -> str:
if self.payload is None:
raise ValueError("Payload passed to notifier is None, cannot build message")

message = []

if self.errors is not None and len(self.errors) > 0:
message.append(specific_error_message(self.errors))
if self.error:
message.append(specific_error_message(self.error))

if self.error and self.payload.info:
message.append("---")

message.append(f"### :x: {self.payload.failed} Tests Failed:")
if self.payload.info:
message.append(f"### :x: {self.payload.failed} Tests Failed:")

completed = self.payload.failed + self.payload.passed
completed = self.payload.failed + self.payload.passed

message += [
"| Tests completed | Failed | Passed | Skipped |",
"|---|---|---|---|",
f"| {completed} | {self.payload.failed} | {self.payload.passed} | {self.payload.skipped} |",
]

failures = sorted(
(
failure
for failure in self.payload.failures
if failure.test_id not in self.payload.flaky_tests
),
key=lambda x: (x.duration_seconds, x.display_name),
)
if failures:
failure_content = [f"{messagify_failure(failure)}" for failure in failures]
message += [
"| Tests completed | Failed | Passed | Skipped |",
"|---|---|---|---|",
f"| {completed} | {self.payload.failed} | {self.payload.passed} | {self.payload.skipped} |",
]

top_3_failed_section = wrap_in_details(
f"View the top {min(3, len(failures))} failed tests by shortest run time",
"\n".join(failure_content),
failures = sorted(
(
failure
for failure in self.payload.info.failures
if failure.test_id not in self.payload.info.flaky_tests
),
key=lambda x: (x.duration_seconds, x.display_name),
)
if failures:
failure_content = [
f"{messagify_failure(failure)}" for failure in failures
]

message.append(top_3_failed_section)
top_3_failed_section = wrap_in_details(
f"View the top {min(3, len(failures))} failed tests by shortest run time",
"\n".join(failure_content),
)

flaky_failures = [
failure
for failure in self.payload.failures
if failure.test_id in self.payload.flaky_tests
]
if flaky_failures:
flake_content = [
f"{messagify_flake(flaky_failure, self.payload.flaky_tests[flaky_failure.test_id])}"
for flaky_failure in flaky_failures
message.append(top_3_failed_section)

flaky_failures = [
failure
for failure in self.payload.info.failures
if failure.test_id in self.payload.info.flaky_tests
]
if flaky_failures:
flake_content = [
f"{messagify_flake(flaky_failure, self.payload.info.flaky_tests[flaky_failure.test_id])}"
for flaky_failure in flaky_failures
]

flaky_section = wrap_in_details(
f"View the full list of {len(flaky_failures)} :snowflake: flaky tests",
"\n".join(flake_content),
)
flaky_section = wrap_in_details(
f"View the full list of {len(flaky_failures)} :snowflake: flaky tests",
"\n".join(flake_content),
)

message.append(flaky_section)
message.append(flaky_section)

message.append(generate_view_test_analytics_line(self.commit))
message.append(generate_view_test_analytics_line(self.commit))
return "\n".join(message)

def error_comment(self):
Expand All @@ -324,12 +334,10 @@ def error_comment(self):
"""
message: str

if self.errors is None:
if self.error is None:
message = ":x: We are unable to process any of the uploaded JUnit XML files. Please ensure your files are in the right format."
else:
if len(self.errors) == 0:
raise ValueError("No errors passed to error_comment method")
message = specific_error_message(self.errors)
message = specific_error_message(self.error)

pull = self.get_pull()
if pull is None:
Expand Down
13 changes: 7 additions & 6 deletions services/tests/test_test_results.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from helpers.notifier import NotifierResult
from services.test_results import (
FlakeInfo,
TACommentInDepthInfo,
TestResultsNotificationFailure,
TestResultsNotificationPayload,
TestResultsNotifier,
Expand Down Expand Up @@ -114,7 +115,8 @@ def test_build_message():
1.0,
"https://example.com/build_url",
)
payload = TestResultsNotificationPayload(1, 2, 3, [fail], dict())
info = TACommentInDepthInfo(failures=[fail], flaky_tests={})
payload = TestResultsNotificationPayload(1, 2, 3, info)
commit = CommitFactory(branch="thing/thing")
tn = TestResultsNotifier(commit, None, None, None, payload)
res = tn.build_message()
Expand Down Expand Up @@ -161,10 +163,9 @@ def test_build_message_with_flake():
1.0,
"https://example.com/build_url",
)

payload = TestResultsNotificationPayload(
1, 2, 3, [fail], {test_id: FlakeInfo(1, 3)}
)
flaky_test = FlakeInfo(1, 3)
info = TACommentInDepthInfo(failures=[fail], flaky_tests={test_id: flaky_test})
payload = TestResultsNotificationPayload(1, 2, 3, info)
commit = CommitFactory(branch="test_branch")
tn = TestResultsNotifier(commit, None, None, None, payload)
res = tn.build_message()
Expand Down Expand Up @@ -296,7 +297,7 @@ def test_specific_error_message(mocker):
"error_message": "Error parsing JUnit XML in test.xml at 4:32: ParserError: No name found"
},
)
tn = TestResultsNotifier(CommitFactory(), None, errors=[error])
tn = TestResultsNotifier(CommitFactory(), None, error=error)
result = tn.error_comment()
expected = """### :x: Unsupported file format
Expand Down
Loading

0 comments on commit c650bbe

Please sign in to comment.