diff options
| author | Aiden Grossman <aidengrossman@google.com> | 2025-11-21 16:52:47 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-11-21 16:52:47 -0800 |
| commit | 99120bb51bf728d7ba7fad5068227f8c6e707159 (patch) | |
| tree | 95681921343141a6e097ea6715d408f4f9e7884e | |
| parent | dc3c5a5ffceb64c5c948e1d22b6d81023bef65b6 (diff) | |
[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).
| -rw-r--r-- | .ci/premerge_advisor_explain.py | 62 |
1 files changed, 30 insertions, 32 deletions
diff --git a/.ci/premerge_advisor_explain.py b/.ci/premerge_advisor_explain.py index 69568895e903..155e91bef55f 100644 --- a/.ci/premerge_advisor_explain.py +++ b/.ci/premerge_advisor_explain.py @@ -79,16 +79,6 @@ def main( pr_number: The number of the PR associated with this run. return_code: The numerical return code of ninja/CMake. """ - if return_code == 0: - with open("comment", "w") as comment_file_handle: - comment = get_comment( - github_token, - pr_number, - ":white_check_mark: With the latest revision this PR passed " - "the premerge checks.", - ) - if "id" in comment: - json.dump([comment], comment_file_handle) junit_objects, ninja_logs = generate_test_report_lib.load_info_from_files( build_log_files ) @@ -105,34 +95,42 @@ def main( explanation_request["failures"].append( {"name": name, "message": failure_messsage} ) - else: + elif return_code != 0: ninja_failures = generate_test_report_lib.find_failure_in_ninja_logs(ninja_logs) for name, failure_message in ninja_failures: explanation_request["failures"].append( {"name": name, "message": failure_message} ) - advisor_response = requests.get( - PREMERGE_ADVISOR_URL, json=explanation_request, timeout=5 + comments = [] + advisor_explanations = [] + if return_code != 0: + advisor_response = requests.get( + PREMERGE_ADVISOR_URL, json=explanation_request, timeout=5 + ) + if advisor_response.status_code == 200: + print(advisor_response.json()) + advisor_explanations = advisor_response.json() + else: + print(advisor_response.reason) + comments.append( + get_comment( + github_token, + pr_number, + generate_test_report_lib.generate_report( + generate_test_report_lib.compute_platform_title(), + return_code, + junit_objects, + ninja_logs, + failure_explanations_list=advisor_explanations, + ), + ) ) - if advisor_response.status_code == 200: - print(advisor_response.json()) - comments = [ - get_comment( - github_token, - pr_number, - generate_test_report_lib.generate_report( - generate_test_report_lib.compute_platform_title(), - return_code, - junit_objects, - ninja_logs, - failure_explanations_list=advisor_response.json(), - ), - ) - ] - with open("comments", "w") as comment_file_handle: - json.dump(comments, comment_file_handle) - else: - print(advisor_response.reason) + if return_code == 0 and "id" not in comments[0]: + # If the job succeeds and there is not an existing comment, we + # should not write one to reduce noise. + comments = [] + with open("comments", "w") as comment_file_handle: + json.dump(comments, comment_file_handle) if __name__ == "__main__": |
