Skip to content

Commit 99120bb

Browse files
[CI] Make Premerge only Comment if Tests Failed (#169102)
Before, we were unconditionally writing a message. After this patch, we only write a message when the tests failed, or there is already an existing comment. This is how this workflow was intended to work originally, but is not how it ended up working, mostly due to my misconceptions around how the existing code formatter pass handled this case (we need to actually not write out any comments, not write out a specific message).
1 parent dc3c5a5 commit 99120bb

File tree

1 file changed

+30
-32
lines changed

1 file changed

+30
-32
lines changed

.ci/premerge_advisor_explain.py

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -79,16 +79,6 @@ def main(
7979
pr_number: The number of the PR associated with this run.
8080
return_code: The numerical return code of ninja/CMake.
8181
"""
82-
if return_code == 0:
83-
with open("comment", "w") as comment_file_handle:
84-
comment = get_comment(
85-
github_token,
86-
pr_number,
87-
":white_check_mark: With the latest revision this PR passed "
88-
"the premerge checks.",
89-
)
90-
if "id" in comment:
91-
json.dump([comment], comment_file_handle)
9282
junit_objects, ninja_logs = generate_test_report_lib.load_info_from_files(
9383
build_log_files
9484
)
@@ -105,34 +95,42 @@ def main(
10595
explanation_request["failures"].append(
10696
{"name": name, "message": failure_messsage}
10797
)
108-
else:
98+
elif return_code != 0:
10999
ninja_failures = generate_test_report_lib.find_failure_in_ninja_logs(ninja_logs)
110100
for name, failure_message in ninja_failures:
111101
explanation_request["failures"].append(
112102
{"name": name, "message": failure_message}
113103
)
114-
advisor_response = requests.get(
115-
PREMERGE_ADVISOR_URL, json=explanation_request, timeout=5
104+
comments = []
105+
advisor_explanations = []
106+
if return_code != 0:
107+
advisor_response = requests.get(
108+
PREMERGE_ADVISOR_URL, json=explanation_request, timeout=5
109+
)
110+
if advisor_response.status_code == 200:
111+
print(advisor_response.json())
112+
advisor_explanations = advisor_response.json()
113+
else:
114+
print(advisor_response.reason)
115+
comments.append(
116+
get_comment(
117+
github_token,
118+
pr_number,
119+
generate_test_report_lib.generate_report(
120+
generate_test_report_lib.compute_platform_title(),
121+
return_code,
122+
junit_objects,
123+
ninja_logs,
124+
failure_explanations_list=advisor_explanations,
125+
),
126+
)
116127
)
117-
if advisor_response.status_code == 200:
118-
print(advisor_response.json())
119-
comments = [
120-
get_comment(
121-
github_token,
122-
pr_number,
123-
generate_test_report_lib.generate_report(
124-
generate_test_report_lib.compute_platform_title(),
125-
return_code,
126-
junit_objects,
127-
ninja_logs,
128-
failure_explanations_list=advisor_response.json(),
129-
),
130-
)
131-
]
132-
with open("comments", "w") as comment_file_handle:
133-
json.dump(comments, comment_file_handle)
134-
else:
135-
print(advisor_response.reason)
128+
if return_code == 0 and "id" not in comments[0]:
129+
# If the job succeeds and there is not an existing comment, we
130+
# should not write one to reduce noise.
131+
comments = []
132+
with open("comments", "w") as comment_file_handle:
133+
json.dump(comments, comment_file_handle)
136134

137135

138136
if __name__ == "__main__":

0 commit comments

Comments
 (0)