[UpdateTestChecks][llvm-mca] Use common.itertests in update_mca_test_checks

[michaelmaitland]

common.itertests has useful functionality such as skipping test case if it starts with UTC_AVOID comment string. It also does some of the functionality, such as skipping test file pattern if not found, that was duplicated by update_mca_test_checks.

This patch uses this function to take advantage of extra behavior and remove duplication.

[llvmbot]

@llvm/pr-subscribers-testing-tools

@llvm/pr-subscribers-tools-llvm-mca

Changes

common.itertests has useful functionality such as skipping test case if it starts with UTC_AVOID comment string. It also does some of the functionality, such as skipping test file pattern if not found, that was duplicated by update_mca_test_checks.

common.parse_commandline_args can be reused as well to avoid duplication.

This patch uses these functions to take advantage of extra behavior and remove duplication.

---

Full diff: https://github.com/llvm/llvm-project/pull/67477.diff

1 Files Affected:

• (modified) llvm/utils/update_mca_test_checks.py (+38-44)

diff --git a/llvm/utils/update_mca_test_checks.py b/llvm/utils/update_mca_test_checks.py
index f7b29fec4efec8f..0efaa5997ed74a7 100755
--- a/llvm/utils/update_mca_test_checks.py
+++ b/llvm/utils/update_mca_test_checks.py
@@ -52,7 +52,7 @@ def _showwarning(message, category, filename, lineno, file=None, line=None):
     file.write(warnings.formatwarning(message, category, filename, lineno, line))
 
 
-def _parse_args():
+def _get_parser():
     parser = argparse.ArgumentParser(description=__doc__)
     parser.add_argument("-w", action="store_true", help="suppress warnings")
     parser.add_argument(
@@ -65,18 +65,7 @@ def _parse_args():
         help="the binary to use to generate the test case " "(default: llvm-mca)",
     )
     parser.add_argument("tests", metavar="<test-path>", nargs="+")
-    args = common.parse_commandline_args(parser)
-
-    _configure_warnings(args)
-
-    if not args.llvm_mca_binary:
-        raise Error("--llvm-mca-binary value cannot be empty string")
-
-    if "llvm-mca" not in os.path.basename(args.llvm_mca_binary):
-        _warn("unexpected binary name: {}".format(args.llvm_mca_binary))
-
-    return args
-
+    return parser
 
 def _get_run_infos(run_lines, args):
     run_infos = []
@@ -546,39 +535,44 @@ def _write_output(
         f.writelines(["{}\n".format(l).encode("utf-8") for l in output_lines])
 
 
-def main():
-    args = _parse_args()
-    test_paths = [test for pattern in args.tests for test in glob.glob(pattern)]
-    for test_path in test_paths:
-        sys.stderr.write("Test: {}\n".format(test_path))
-
-        # Call this per test. By default each warning will only be written once
-        # per source location. Reset the warning filter so that now each warning
-        # will be written once per source location per test.
-        _configure_warnings(args)
-
-        if not os.path.isfile(test_path):
-            raise Error("could not find test file: {}".format(test_path))
-
-        with open(test_path) as f:
-            input_lines = [l.rstrip() for l in f]
-
-        run_lines = common.find_run_lines(test_path, input_lines)
-        run_infos = _get_run_infos(run_lines, args)
-        common_prefix, prefix_pad = _get_useful_prefix_info(run_infos)
-        block_infos = _get_block_infos(run_infos, test_path, args, common_prefix)
-        _write_output(
-            test_path,
-            input_lines,
-            run_infos,
-            block_infos,
-            args,
-            common_prefix,
-            prefix_pad,
-        )
+def update_test_file(args, test_path, autogenerated_note):
+    sys.stderr.write("Test: {}\n".format(test_path))
 
-    return 0
+    # Call this per test. By default each warning will only be written once
+    # per source location. Reset the warning filter so that now each warning
+    # will be written once per source location per test.
+    _configure_warnings(args)
+
+    with open(test_path) as f:
+        input_lines = [l.rstrip() for l in f]
+
+    run_lines = common.find_run_lines(test_path, input_lines)
+    run_infos = _get_run_infos(run_lines, args)
+    common_prefix, prefix_pad = _get_useful_prefix_info(run_infos)
+    block_infos = _get_block_infos(run_infos, test_path, args, common_prefix)
+    _write_output(
+        test_path,
+        input_lines,
+        run_infos,
+        block_infos,
+        args,
+        common_prefix,
+        prefix_pad,
+    )
 
+def main():
+    script_name = "utils/" + os.path.basename(__file__)
+    parser = _get_parser()
+    args = common.parse_commandline_args(parser)
+    for ti in common.itertests(
+        args.tests, parser, script_name=script_name
+    ):
+        try:
+            update_test_file(ti.args, ti.path, ti.test_autogenerated_note)
+        except Exception:
+            common.warn("Error processing file", test_file=ti.path)
+            raise
+    return 0
 
 if __name__ == "__main__":
     try:

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[mshockwave]

I think we should also preserve the checks from line 72 to 76

[michaelmaitland]

Updated, but moved to main since parse_commandline_args is called in main.

[mshockwave]

LGTM