Skip to content

Commit 5c9d6ad

Browse files
boomanaiden154vinay-deshmukh
boomanaiden154
authored and
vinay-deshmukh
committed
[CI] Add Ability to Explain Failures
With the premerge advisor infrastructure almost done, we can now request on demand explanations (failing at head or flaky). This patch adds the infrastructure to write out test reports containing this information so we can easily surface it to the user. Reviewers: Keenuts, gburgessiv, dschuff, lnihlen Reviewed By: Keenuts Pull Request: llvm#166590
1 parent 0878d09 commit 5c9d6ad

File tree

2 files changed

+192
-7
lines changed

2 files changed

+192
-7
lines changed

‎.ci/generate_test_report_lib.py‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,19 @@
33
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44
"""Library to parse JUnit XML files and return a markdown report."""
55

6+
from typing import TypedDict
7+
68
from junitparser import JUnitXml, Failure
79

10+
11+
# This data structure should match the definition in llvm-zorg in
12+
# premerge/advisor/advisor_lib.py
13+
class FailureExplanation(TypedDict):
14+
name: str
15+
explained: bool
16+
reason: str | None
17+
18+
819
SEE_BUILD_FILE_STR = "Download the build's log file to see the details."
920
UNRELATED_FAILURES_STR = (
1021
"If these failures are unrelated to your changes (for example "
@@ -82,16 +93,29 @@ def find_failure_in_ninja_logs(ninja_logs: list[list[str]]) -> list[tuple[str, s
8293
return failures
8394

8495

85-
def _format_failures(failures: list[tuple[str, str]]) -> list[str]:
96+
def _format_failures(
97+
failures: list[tuple[str, str]], failure_explanations: dict[str, FailureExplanation]
98+
) -> list[str]:
8699
"""Formats failures into summary views for the report."""
87100
output = []
88101
for build_failure in failures:
89102
failed_action, failure_message = build_failure
103+
failure_explanation = None
104+
if failed_action in failure_explanations:
105+
failure_explanation = failure_explanations[failed_action]
106+
output.append("<details>")
107+
if failure_explanation:
108+
output.extend(
109+
[
110+
f"<summary>{failed_action} (Likely Already Failing)</summary>" "",
111+
failure_explanation["reason"],
112+
"",
113+
]
114+
)
115+
else:
116+
output.extend([f"<summary>{failed_action}</summary>", ""])
90117
output.extend(
91118
[
92-
"<details>",
93-
f"<summary>{failed_action}</summary>",
94-
"",
95119
"```",
96120
failure_message,
97121
"```",
@@ -132,12 +156,19 @@ def generate_report(
132156
ninja_logs: list[list[str]],
133157
size_limit=1024 * 1024,
134158
list_failures=True,
159+
failure_explanations_list: list[FailureExplanation] = [],
135160
):
136161
failures = get_failures(junit_objects)
137162
tests_run = 0
138163
tests_skipped = 0
139164
tests_failed = 0
140165

166+
failure_explanations: dict[str, FailureExplanation] = {}
167+
for failure_explanation in failure_explanations_list:
168+
if not failure_explanation["explained"]:
169+
continue
170+
failure_explanations[failure_explanation["name"]] = failure_explanation
171+
141172
for results in junit_objects:
142173
for testsuite in results:
143174
tests_run += testsuite.tests
@@ -176,7 +207,7 @@ def generate_report(
176207
"",
177208
]
178209
)
179-
report.extend(_format_failures(ninja_failures))
210+
report.extend(_format_failures(ninja_failures, failure_explanations))
180211
report.extend(
181212
[
182213
"",
@@ -212,7 +243,7 @@ def plural(num_tests):
212243

213244
for testsuite_name, failures in failures.items():
214245
report.extend(["", f"### {testsuite_name}"])
215-
report.extend(_format_failures(failures))
246+
report.extend(_format_failures(failures, failure_explanations))
216247
elif return_code != 0:
217248
# No tests failed but the build was in a failed state. Bring this to the user's
218249
# attention.
@@ -237,7 +268,7 @@ def plural(num_tests):
237268
"",
238269
]
239270
)
240-
report.extend(_format_failures(ninja_failures))
271+
report.extend(_format_failures(ninja_failures, failure_explanations))
241272

242273
if failures or return_code != 0:
243274
report.extend(["", UNRELATED_FAILURES_STR])

‎.ci/generate_test_report_lib_test.py‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,160 @@ def test_report_size_limit(self):
781781
),
782782
)
783783

784+
def test_report_ninja_explanation(self):
785+
self.assertEqual(
786+
generate_test_report_lib.generate_report(
787+
"Foo",
788+
1,
789+
[],
790+
[
791+
[
792+
"[1/5] test/1.stamp",
793+
"[2/5] test/2.stamp",
794+
"[3/5] test/3.stamp",
795+
"[4/5] test/4.stamp",
796+
"FAILED: test/4.stamp",
797+
"touch test/4.stamp",
798+
"Half Moon Bay.",
799+
"[5/5] test/5.stamp",
800+
]
801+
],
802+
failure_explanations_list=[
803+
{
804+
"name": "test/4.stamp",
805+
"explained": True,
806+
"reason": "Failing at head",
807+
}
808+
],
809+
),
810+
dedent(
811+
"""\
812+
# Foo
813+
814+
The build failed before running any tests. Click on a failure below to see the details.
815+
816+
<details>
817+
<summary>test/4.stamp (Likely Already Failing)</summary>
818+
Failing at head
819+
820+
```
821+
FAILED: test/4.stamp
822+
touch test/4.stamp
823+
Half Moon Bay.
824+
```
825+
</details>
826+
827+
If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the `infrastructure` label."""
828+
),
829+
)
830+
831+
def test_report_test_failure_explanation(self):
832+
self.assertEqual(
833+
generate_test_report_lib.generate_report(
834+
"Foo",
835+
1,
836+
[
837+
junit_from_xml(
838+
dedent(
839+
"""\
840+
<?xml version="1.0" encoding="UTF-8"?>
841+
<testsuites time="8.89">
842+
<testsuite name="Bar" tests="1" failures="1" skipped="0" time="410.63">
843+
<testcase classname="Bar/test_3" name="test_3" time="0.02">
844+
<failure><![CDATA[Error! Expected Big Sur to be next to the ocean.]]></failure>
845+
</testcase>
846+
</testsuite>
847+
</testsuites>"""
848+
)
849+
)
850+
],
851+
[],
852+
failure_explanations_list=[
853+
{
854+
"name": "Bar/test_3/test_3",
855+
"explained": True,
856+
"reason": "Big Sur is next to the Pacific.",
857+
}
858+
],
859+
),
860+
(
861+
dedent(
862+
"""\
863+
# Foo
864+
865+
* 1 test failed
866+
867+
## Failed Tests
868+
(click on a test name to see its output)
869+
870+
### Bar
871+
<details>
872+
<summary>Bar/test_3/test_3 (Likely Already Failing)</summary>
873+
Big Sur is next to the Pacific.
874+
875+
```
876+
Error! Expected Big Sur to be next to the ocean.
877+
```
878+
</details>
879+
880+
If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the `infrastructure` label."""
881+
)
882+
),
883+
)
884+
885+
def test_report_test_failure_have_explanation_explained_false(self):
886+
self.assertEqual(
887+
generate_test_report_lib.generate_report(
888+
"Foo",
889+
1,
890+
[
891+
junit_from_xml(
892+
dedent(
893+
"""\
894+
<?xml version="1.0" encoding="UTF-8"?>
895+
<testsuites time="8.89">
896+
<testsuite name="Bar" tests="1" failures="1" skipped="0" time="410.63">
897+
<testcase classname="Bar/test_3" name="test_3" time="0.02">
898+
<failure><![CDATA[Error! Expected Mt. Shasta to be next in the Eastern Sierras.]]></failure>
899+
</testcase>
900+
</testsuite>
901+
</testsuites>"""
902+
)
903+
)
904+
],
905+
[],
906+
failure_explanations_list=[
907+
{
908+
"name": "Bar/test_3/test_3",
909+
"explained": False,
910+
"reason": "Mt. Shasta is in the Cascades",
911+
}
912+
],
913+
),
914+
(
915+
dedent(
916+
"""\
917+
# Foo
918+
919+
* 1 test failed
920+
921+
## Failed Tests
922+
(click on a test name to see its output)
923+
924+
### Bar
925+
<details>
926+
<summary>Bar/test_3/test_3</summary>
927+
928+
```
929+
Error! Expected Mt. Shasta to be next in the Eastern Sierras.
930+
```
931+
</details>
932+
933+
If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the `infrastructure` label."""
934+
)
935+
),
936+
)
937+
784938
def test_generate_report_end_to_end(self):
785939
with tempfile.TemporaryDirectory() as temp_dir:
786940
junit_xml_file = os.path.join(temp_dir, "junit.xml")

0 commit comments

Comments
 (0)