llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-infrastructure Author: Aiden Grossman (boomanaiden154) <details> <summary>Changes</summary> This will mark the CI as green if the premerge advisor is able to explain all of the failures. We have seen many false negatives (failures not explained that should be), but no false positives (failures explained that should not be) so far. --- Full diff: https://github.com/llvm/llvm-project/pull/172394.diff 3 Files Affected: - (modified) .ci/generate_test_report_lib.py (+27-3) - (modified) .ci/generate_test_report_lib_test.py (+52-24) - (modified) .ci/premerge_advisor_explain.py (+13-14) ``````````diff diff --git a/.ci/generate_test_report_lib.py b/.ci/generate_test_report_lib.py index 5edde254eb73d..734ce3a4f4316 100644 --- a/.ci/generate_test_report_lib.py +++ b/.ci/generate_test_report_lib.py @@ -158,6 +158,16 @@ def get_failures(junit_objects) -> dict[str, list[tuple[str, str]]]: return failures +def are_all_failures_explained( + failures: list[tuple[str, str]], failure_explanations: dict[str, FailureExplanation] +) -> bool: + for failure in failures: + failed_action, _ = failure + if failed_action not in failure_explanations: + return False + return True + + # Set size_limit to limit the byte size of the report. The default is 1MB as this # is the most that can be put into an annotation. If the generated report exceeds # this limit and failures are listed, it will be generated again without failures @@ -172,7 +182,7 @@ def generate_report( size_limit=1024 * 1024, list_failures=True, failure_explanations_list: list[FailureExplanation] = [], -): +) -> tuple[str, bool]: failures = get_failures(junit_objects) tests_run = 0 tests_skipped = 0 @@ -183,6 +193,12 @@ def generate_report( if not failure_explanation["explained"]: continue failure_explanations[failure_explanation["name"]] = failure_explanation + all_failures_explained = True + if failures: + for _, failures_list in failures.items(): + all_failures_explained &= are_all_failures_explained( + failures_list, failure_explanations + ) for results in junit_objects: for testsuite in results: @@ -202,7 +218,11 @@ def generate_report( ) else: ninja_failures = find_failure_in_ninja_logs(ninja_logs) + all_failures_explained &= are_all_failures_explained( + ninja_failures, failure_explanations + ) if not ninja_failures: + all_failures_explained = False report.extend( [ "The build failed before running any tests. Detailed " @@ -229,7 +249,7 @@ def generate_report( UNRELATED_FAILURES_STR, ] ) - return "\n".join(report) + return ("\n".join(report), all_failures_explained) tests_passed = tests_run - tests_skipped - tests_failed @@ -263,7 +283,11 @@ def plural(num_tests): # No tests failed but the build was in a failed state. Bring this to the user's # attention. ninja_failures = find_failure_in_ninja_logs(ninja_logs) + all_failures_explained &= are_all_failures_explained( + ninja_failures, failure_explanations + ) if not ninja_failures: + all_failures_explained = False report.extend( [ "", @@ -302,7 +326,7 @@ def plural(num_tests): list_failures=False, ) - return report + return (report, all_failures_explained) def load_info_from_files(build_log_files): diff --git a/.ci/generate_test_report_lib_test.py b/.ci/generate_test_report_lib_test.py index 06279d672f3c3..bd72a7b2314d5 100644 --- a/.ci/generate_test_report_lib_test.py +++ b/.ci/generate_test_report_lib_test.py @@ -191,19 +191,23 @@ def test_ninja_log_mismatched_failed(self): def test_title_only(self): self.assertEqual( generate_test_report_lib.generate_report("Foo", 0, [], []), - dedent( - """\ + ( + dedent( + """\ # Foo :white_check_mark: The build succeeded and no tests ran. This is expected in some build configurations.""" + ), + True, ), ) def test_title_only_failure(self): self.assertEqual( generate_test_report_lib.generate_report("Foo", 1, [], []), - dedent( - """\ + ( + dedent( + """\ # Foo The build failed before running any tests. Detailed information about the build failure could not be automatically obtained. @@ -211,6 +215,8 @@ def test_title_only_failure(self): Download the build's log file to see the details. 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.""" + ), + False, ), ) @@ -233,8 +239,9 @@ def test_title_only_failure_ninja_log(self): ] ], ), - dedent( - """\ + ( + dedent( + """\ # Foo The build failed before running any tests. Click on a failure below to see the details. @@ -250,6 +257,8 @@ def test_title_only_failure_ninja_log(self): </details> 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.""" + ), + False, ), ) @@ -272,8 +281,9 @@ def test_no_tests_in_testsuite(self): ], [], ), - dedent( - """\ + ( + dedent( + """\ # Foo The build failed before running any tests. Detailed information about the build failure could not be automatically obtained. @@ -281,6 +291,8 @@ def test_no_tests_in_testsuite(self): Download the build's log file to see the details. 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.""" + ), + False, ), ) @@ -312,7 +324,8 @@ def test_no_failures(self): * 1 test passed :white_check_mark: The build succeeded and all tests passed.""" - ) + ), + True, ), ) @@ -348,7 +361,8 @@ def test_no_failures_build_failed(self): Download the build's log file to see the details. 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.""" - ) + ), + False, ), ) @@ -403,7 +417,8 @@ def test_no_failures_build_failed_ninja_log(self): </details> 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.""" - ) + ), + False, ), ) @@ -496,7 +511,8 @@ def test_no_failures_multiple_build_failed_ninja_log(self): </details> 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.""" - ) + ), + False, ), ) @@ -558,7 +574,8 @@ def test_report_single_file_single_testsuite(self): </details> 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.""" - ) + ), + False, ), ) @@ -625,7 +642,7 @@ def test_report_single_file_multiple_testsuites(self): ], [], ), - self.MULTI_SUITE_OUTPUT, + (self.MULTI_SUITE_OUTPUT, False), ) def test_report_multiple_files_multiple_testsuites(self): @@ -667,7 +684,7 @@ def test_report_multiple_files_multiple_testsuites(self): ], [], ), - self.MULTI_SUITE_OUTPUT, + (self.MULTI_SUITE_OUTPUT, False), ) def test_report_dont_list_failures(self): @@ -703,7 +720,8 @@ def test_report_dont_list_failures(self): Failed tests and their output was too large to report. Download the build's log file to see the details. 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.""" - ) + ), + False, ), ) @@ -740,7 +758,8 @@ def test_report_dont_list_failures_link_to_log(self): Failed tests and their output was too large to report. Download the build's log file to see the details. 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.""" - ) + ), + False, ), ) @@ -780,7 +799,8 @@ def test_report_size_limit(self): Failed tests and their output was too large to report. Download the build's log file to see the details. 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.""" - ) + ), + False, ), ) @@ -810,8 +830,9 @@ def test_report_ninja_explanation(self): } ], ), - dedent( - """\ + ( + dedent( + """\ # Foo The build failed before running any tests. Click on a failure below to see the details. @@ -828,6 +849,8 @@ def test_report_ninja_explanation(self): </details> 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.""" + ), + True, ), ) @@ -881,7 +904,8 @@ def test_report_test_failure_explanation(self): </details> 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.""" - ) + ), + True, ), ) @@ -934,7 +958,8 @@ def test_report_test_failure_have_explanation_explained_false(self): </details> 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.""" - ) + ), + False, ), ) @@ -972,8 +997,9 @@ def test_generate_report_end_to_end(self): generate_test_report_lib.generate_report_from_files( "Foo", 1, [junit_xml_file, ninja_log_file] ), - dedent( - """\ + ( + dedent( + """\ # Foo * 1 test passed @@ -991,5 +1017,7 @@ def test_generate_report_end_to_end(self): </details> 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.""" + ), + False, ), ) diff --git a/.ci/premerge_advisor_explain.py b/.ci/premerge_advisor_explain.py index 00df1b94c4578..3da3d579de221 100644 --- a/.ci/premerge_advisor_explain.py +++ b/.ci/premerge_advisor_explain.py @@ -54,7 +54,7 @@ def main( github_token: str, pr_number: int, return_code: int, -): +) -> bool: """The main entrypoint for the script. This function parses failures from files, requests information from the @@ -113,19 +113,14 @@ def main( 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, - ), - ) + report, failures_explained = generate_test_report_lib.generate_report( + generate_test_report_lib.compute_platform_title(), + return_code, + junit_objects, + ninja_logs, + failure_explanations_list=advisor_explanations, ) + comments.append(get_comment(github_token, pr_number, report)) 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. @@ -134,6 +129,7 @@ def main( with open(comments_file_name, "w") as comment_file_handle: json.dump(comments, comment_file_handle) print(f"Wrote comments to {comments_file_name}") + return failures_explained if __name__ == "__main__": @@ -152,7 +148,7 @@ def main( if platform.machine() == "arm64" or platform.machine() == "aarch64": sys.exit(0) - main( + failures_explained = main( args.commit_sha, args.build_log_files, args.github_token, @@ -160,4 +156,7 @@ def main( args.return_code, ) + if failures_explained: + sys.exit(0) + sys.exit(args.return_code) `````````` </details> https://github.com/llvm/llvm-project/pull/172394 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
