Refactor lit test infra under //testing. (#2829)

- Moves most parts to //testing/lit_test to be consistent with //testing/file_test.
- Separates the autoupdate script out because it's shared between lit_test and file_test now, not lit-specific.
- Renames scripts to autoupdate_testdata (or autoupdate_lit_testdata for explorer's extra) to be more consistent with the non-lit-specific setup.
  - Switches from execv to subprocess.call to head off a subtle issue regarding execution of multiple scripts, which we're likely to want in the future. Mostly in this PR because everything was already being touched.
- Removes autoupdate's dependency on merge_output in order to (a) better support the division of lit and non-lit logic and (b) remove a subprocess, for reasons similar to file_test's removal of subprocesses.

explorer/lit_autoupdate_lit.py → explorer/autoupdate_lit_testdata.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-"""Updates the CHECK: lines in lit tests based on the AUTOUPDATE line."""
+"""Updates the CHECK: lines in tests with an AUTOUPDATE line."""
 
 __copyright__ = """
 Part of the Carbon Language project, under the Apache License v2.0 with LLVM
@@ -8,27 +8,26 @@ Exceptions. See /LICENSE for license information.
 SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
-import os
+import subprocess
 import sys
 from pathlib import Path
 
 
 def main() -> None:
-    # Calls the main script using execv in order to avoid Python import
-    # behaviors.
+    # Subprocess to the main script in order to avoid Python import behaviors.
     this_py = Path(__file__).resolve()
-    actual_py = this_py.parent.parent.joinpath(
-        "bazel", "testing", "lit_autoupdate_base.py"
+    autoupdate_py = this_py.parent.parent.joinpath(
+        "testing", "scripts", "autoupdate_testdata_base.py"
     )
     args = [
-        sys.argv[0],
+        str(autoupdate_py),
         # Flags to configure for explorer testing.
         "--tool=explorer",
         "--testdata=explorer/lit_testdata",
         "--lit_run=%{explorer-run}",
         "--lit_run=%{explorer-run-trace}",
     ] + sys.argv[1:]
-    os.execv(actual_py, args)
+    exit(subprocess.call(args))
 
 
 if __name__ == "__main__":

explorer/lit_autoupdate.py → explorer/autoupdate_testdata.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-"""Updates the CHECK: lines in lit tests based on the AUTOUPDATE line."""
+"""Updates the CHECK: lines in tests with an AUTOUPDATE line."""
 
 __copyright__ = """
 Part of the Carbon Language project, under the Apache License v2.0 with LLVM
@@ -8,25 +8,24 @@ Exceptions. See /LICENSE for license information.
 SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
-import os
+import subprocess
 import sys
 from pathlib import Path
 
 
 def main() -> None:
-    # Calls the main script using execv in order to avoid Python import
-    # behaviors.
+    # Subprocess to the main script in order to avoid Python import behaviors.
     this_py = Path(__file__).resolve()
-    actual_py = this_py.parent.parent.joinpath(
-        "bazel", "testing", "lit_autoupdate_base.py"
+    autoupdate_py = this_py.parent.parent.joinpath(
+        "testing", "scripts", "autoupdate_testdata_base.py"
     )
     args = [
-        sys.argv[0],
+        str(autoupdate_py),
         # Flags to configure for explorer testing.
         "--tool=explorer",
         "--testdata=explorer/testdata",
     ] + sys.argv[1:]
-    os.execv(actual_py, args)
+    exit(subprocess.call(args))
 
 
 if __name__ == "__main__":

explorer/lit_testdata/BUILD

@@ -3,12 +3,12 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 load("//bazel/sh_run:rules.bzl", "glob_sh_run")
-load("//bazel/testing:lit.bzl", "glob_lit_tests")
+load("//testing/lit_test:rules.bzl", "glob_lit_tests")
 
 glob_lit_tests(
     data = [
-        "//bazel/testing:merge_output",
         "//explorer",
+        "//testing/lit_test:merge_output",
         "@llvm-project//llvm:FileCheck",
         "@llvm-project//llvm:not",
     ],

explorer/lit_testdata/lit.cfg.py

@@ -1 +1 @@
-../../bazel/testing/lit.cfg.py
+../../testing/lit_test/lit.cfg.py

bazel/testing/BUILD → testing/lit_test/BUILD

@@ -4,10 +4,7 @@
 
 package(default_visibility = ["//visibility:public"])
 
-exports_files([
-    "lit_test.py",
-    "merge_output.py",
-])
+exports_files(["lit_test.py"])
 
 py_binary(
     name = "merge_output",

bazel/testing/lit.cfg.py → testing/lit_test/lit.cfg.py

@@ -30,7 +30,7 @@ def add_substitutions():
         "explorer_prelude": fullpath("carbon/explorer/data/prelude.carbon"),
         "filecheck": fullpath("llvm-project/llvm/FileCheck"),
         "not": fullpath("llvm-project/llvm/not"),
-        "merge_output": fullpath("carbon/bazel/testing/merge_output"),
+        "merge_output": fullpath("carbon/testing/lit_test/merge_output"),
     }
 
     run_carbon = f"{tools['merge_output']} {tools['carbon']}"

bazel/testing/merge_output.py → testing/lit_test/merge_output.py

@@ -10,13 +10,16 @@ import subprocess
 import sys
 
 
-def _print(output: str, label: str) -> None:
+def label_output(label: str, output: str) -> None:
+    """Prints output with labels.
+
+    This mirrors label_output in scripts/autoupdate_testdata_base.py and should
+    be kept in sync. They're separate in order to avoid a subprocess or import
+    complexity.
+    """
     if output:
         for line in output.splitlines():
-            if line:
-                print(f"{label}: {line}")
-            else:
-                print(f"{label}:")
+            print(" ".join(filter(None, (label, line))))
 
 
 def main() -> None:
@@ -26,9 +29,8 @@ def main() -> None:
         stderr=subprocess.PIPE,
         encoding="utf-8",
     )
-    # The `lambda line` forces prefixes on empty lines.
-    _print(p.stdout, "STDOUT")
-    _print(p.stderr, "STDERR")
+    label_output("STDOUT:", p.stdout)
+    label_output("STDERR:", p.stderr)
     exit(p.returncode)
 
 

bazel/testing/lit.bzl → testing/lit_test/rules.bzl

@@ -33,8 +33,8 @@ def glob_lit_tests(driver, data, test_file_exts, exclude = None, **kwargs):
         test = "%s.test" % f
         native.py_test(
             name = test,
-            srcs = ["//bazel/testing:lit_test.py"],
-            main = "//bazel/testing:lit_test.py",
+            srcs = ["//testing/lit_test:lit_test.py"],
+            main = "//testing/lit_test:lit_test.py",
             data = data + [driver, f],
             args = ["--package_name=%s" % native.package_name(), "--"],
             size = "small",

bazel/testing/lit_autoupdate_base.py → testing/scripts/autoupdate_testdata_base.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-"""Updates the CHECK: lines in lit tests based on the AUTOUPDATE line."""
+"""Updates the CHECK: lines in tests with an AUTOUPDATE line."""
 
 __copyright__ = """
 Part of the Carbon Language project, under the Apache License v2.0 with LLVM
@@ -33,24 +33,10 @@ AUTOUPDATE_MARKER = "// AUTOUPDATE"
 # Indicates no autoupdate is requested.
 NOAUTOUPDATE_MARKER = "// NOAUTOUPDATE"
 
-# Standard replacements normally done in lit.cfg.py.
-MERGE_OUTPUT = "./bazel-bin/bazel/testing/merge_output"
-
-
-class Tool(NamedTuple):
-    build_target: str
-    autoupdate_cmd: List[str]
-
-
-tools = {
-    "carbon": Tool(
-        "//toolchain/driver:carbon",
-        [MERGE_OUTPUT, "./bazel-bin/toolchain/driver/carbon"],
-    ),
-    "explorer": Tool(
-        "//explorer",
-        [MERGE_OUTPUT, "./bazel-bin/explorer/explorer"],
-    ),
+# Supported tools.
+TOOLS = {
+    "carbon": "//toolchain/driver:carbon",
+    "explorer": "//explorer:explorer",
 }
 
 
@@ -127,7 +113,7 @@ def parse_args() -> ParsedArgs:
         "--tool",
         metavar="TOOL",
         required=True,
-        choices=tools.keys(),
+        choices=TOOLS.keys(),
         help="The tool being tested.",
     )
     parsed_args = parser.parse_args()
@@ -227,8 +213,7 @@ class CheckLine(Line):
     def format(
         self, *, output_line_number: int, line_number_remap: Dict[int, int]
     ) -> str:
-        if not self.out_line:
-            return f"{self.indent}// CHECK-EMPTY:\n"
+        assert self.out_line
         result = self.out_line
         while True:
             match = self.line_number_pattern.search(result)
@@ -293,6 +278,20 @@ def replace_all(s: str, replacements: List[Tuple[str, str]]) -> str:
     return s
 
 
+def label_output(label: str, output: str) -> List[str]:
+    """Merges output with labels.
+
+    This mirrors label_output in lit_test/merge_output.py and should
+    be kept in sync. They're separate in order to avoid a subprocess or import
+    complexity.
+    """
+    result = []
+    if output:
+        for line in output.splitlines():
+            result.append(" ".join(filter(None, (label, line))))
+    return result
+
+
 def get_matchable_test_output(
     autoupdate_args: List[str],
     for_lit: bool,
@@ -305,31 +304,36 @@ def get_matchable_test_output(
     """Runs the autoupdate command and returns the output lines."""
     # Run the autoupdate command to generate output.
     # (`bazel run` would serialize)
-    out = subprocess.run(
-        tools[tool].autoupdate_cmd + autoupdate_args + [test],
+    autoupdate_cmd = TOOLS[tool].replace("//", "./bazel-bin/").replace(":", "/")
+    p = subprocess.run(
+        [autoupdate_cmd] + autoupdate_args + [test],
         env={"LLVM_SYMBOLIZER_PATH": llvm_symbolizer},
         stdout=subprocess.PIPE,
-        stderr=subprocess.STDOUT,
+        stderr=subprocess.PIPE,
         encoding="utf-8",
-    ).stdout
-
-    # Escape things that mirror FileCheck special characters.
-    out = out.replace("{{", "{{[{][{]}}")
-    out = out.replace("[[", "{{[[][[]}}")
-    if for_lit:
-        # `lit` uses full paths to the test file, so use a regex to ignore paths
-        # when used.
-        out = out.replace(test, f"{{{{.*}}}}/{test}")
-        out = bazel_runfiles.sub("{{.*}}/", out)
-    else:
-        # When not using `lit`, the runfiles path is removed.
-        out = bazel_runfiles.sub("", out)
-    out_lines = out.splitlines()
+    )
+
+    out_lines = label_output("STDOUT:", p.stdout)
+    out_lines.extend(label_output("STDERR:", p.stderr))
 
     for i, line in enumerate(out_lines):
+        # Escape things that mirror FileCheck special characters.
+        line = line.replace("{{", "{{[{][{]}}")
+        line = line.replace("[[", "{{[[][[]}}")
+        if for_lit:
+            # `lit` uses full paths to the test file, so use a regex to ignore
+            # paths when used.
+            line = line.replace(test, f"{{{{.*}}}}/{test}")
+            line = bazel_runfiles.sub("{{.*}}/", line)
+        else:
+            # When not using `lit`, the runfiles path is removed.
+            line = bazel_runfiles.sub("", line)
+
         for line_matcher, before, after in extra_check_replacements:
             if line_matcher.match(line):
-                out_lines[i] = before.sub(after, line)
+                line = before.sub(after, line)
+
+        out_lines[i] = line
 
     return out_lines
 
@@ -508,8 +512,7 @@ def main() -> None:
             "build",
             "-c",
             parsed_args.build_mode,
-            "//bazel/testing:merge_output",
-            tools[parsed_args.tool].build_target,
+            TOOLS[parsed_args.tool],
         ]
     )
     bazel_bin_dir = subprocess.check_output(

toolchain/driver/testdata/BUILD

@@ -2,11 +2,11 @@
 # Exceptions. See /LICENSE for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-load("//bazel/testing:lit.bzl", "glob_lit_tests")
+load("//testing/lit_test:rules.bzl", "glob_lit_tests")
 
 glob_lit_tests(
     data = [
-        "//bazel/testing:merge_output",
+        "//testing/lit_test:merge_output",
         "//toolchain/driver:carbon",
         "@llvm-project//llvm:FileCheck",
         "@llvm-project//llvm:not",

toolchain/driver/testdata/lit.cfg.py

@@ -1 +1 @@
-../../../bazel/testing/lit.cfg.py
+../../../testing/lit_test/lit.cfg.py

toolchain/lexer/lit_autoupdate.py → toolchain/lexer/autoupdate_testdata.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-"""Updates the CHECK: lines in lit tests based on the AUTOUPDATE line."""
+"""Updates the CHECK: lines in tests with an AUTOUPDATE line."""
 
 __copyright__ = """
 Part of the Carbon Language project, under the Apache License v2.0 with LLVM
@@ -8,20 +8,19 @@ Exceptions. See /LICENSE for license information.
 SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
-import os
+import subprocess
 import sys
 from pathlib import Path
 
 
 def main() -> None:
-    # Calls the main script using execv in order to avoid Python import
-    # behaviors.
+    # Subprocess to the main script in order to avoid Python import behaviors.
     this_py = Path(__file__).resolve()
-    actual_py = this_py.parent.parent.parent.joinpath(
-        "bazel", "testing", "lit_autoupdate_base.py"
+    autoupdate_py = this_py.parent.parent.parent.joinpath(
+        "testing", "scripts", "autoupdate_testdata_base.py"
     )
     args = [
-        sys.argv[0],
+        str(autoupdate_py),
         # Flags to configure for lexer testing.
         "--tool=carbon",
         "--autoupdate_arg=dump",
@@ -38,7 +37,7 @@ def main() -> None:
         r"(?P<prefix> line: )(?P<line> *\d+)(?P<suffix>,)",
         "--testdata=toolchain/lexer/testdata",
     ] + sys.argv[1:]
-    os.execv(actual_py, args)
+    exit(subprocess.call(args))
 
 
 if __name__ == "__main__":

toolchain/lowering/lit_autoupdate.py → toolchain/lowering/autoupdate_testdata.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-"""Updates the CHECK: lines in lit tests based on the AUTOUPDATE line."""
+"""Updates the CHECK: lines in tests with an AUTOUPDATE line."""
 
 __copyright__ = """
 Part of the Carbon Language project, under the Apache License v2.0 with LLVM
@@ -8,27 +8,26 @@ Exceptions. See /LICENSE for license information.
 SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
-import os
+import subprocess
 import sys
 from pathlib import Path
 
 
 def main() -> None:
-    # Calls the main script using execv in order to avoid Python import
-    # behaviors.
+    # Subprocess to the main script in order to avoid Python import behaviors.
     this_py = Path(__file__).resolve()
-    actual_py = this_py.parent.parent.parent.joinpath(
-        "bazel", "testing", "lit_autoupdate_base.py"
+    autoupdate_py = this_py.parent.parent.parent.joinpath(
+        "testing", "scripts", "autoupdate_testdata_base.py"
     )
     args = [
-        sys.argv[0],
+        str(autoupdate_py),
         # Flags to configure for lowering testing.
         "--tool=carbon",
         "--autoupdate_arg=dump",
         "--autoupdate_arg=llvm-ir",
         "--testdata=toolchain/lowering/testdata",
     ] + sys.argv[1:]
-    os.execv(actual_py, args)
+    exit(subprocess.call(args))
 
 
 if __name__ == "__main__":

toolchain/parser/lit_autoupdate.py → toolchain/parser/autoupdate_testdata.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-"""Updates the CHECK: lines in lit tests based on the AUTOUPDATE line."""
+"""Updates the CHECK: lines in tests with an AUTOUPDATE line."""
 
 __copyright__ = """
 Part of the Carbon Language project, under the Apache License v2.0 with LLVM
@@ -8,27 +8,26 @@ Exceptions. See /LICENSE for license information.
 SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
-import os
+import subprocess
 import sys
 from pathlib import Path
 
 
 def main() -> None:
-    # Calls the main script using execv in order to avoid Python import
-    # behaviors.
+    # Subprocess to the main script in order to avoid Python import behaviors.
     this_py = Path(__file__).resolve()
-    actual_py = this_py.parent.parent.parent.joinpath(
-        "bazel", "testing", "lit_autoupdate_base.py"
+    autoupdate_py = this_py.parent.parent.parent.joinpath(
+        "testing", "scripts", "autoupdate_testdata_base.py"
     )
     args = [
-        sys.argv[0],
+        str(autoupdate_py),
         # Flags to configure for parser testing.
         "--tool=carbon",
         "--autoupdate_arg=dump",
         "--autoupdate_arg=parse-tree",
         "--testdata=toolchain/parser/testdata",
     ] + sys.argv[1:]
-    os.execv(actual_py, args)
+    exit(subprocess.call(args))
 
 
 if __name__ == "__main__":

toolchain/semantics/lit_autoupdate.py → toolchain/semantics/autoupdate_testdata.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-"""Updates the CHECK: lines in lit tests based on the AUTOUPDATE line."""
+"""Updates the CHECK: lines in tests with an AUTOUPDATE line."""
 
 __copyright__ = """
 Part of the Carbon Language project, under the Apache License v2.0 with LLVM
@@ -8,27 +8,26 @@ Exceptions. See /LICENSE for license information.
 SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
-import os
+import subprocess
 import sys
 from pathlib import Path
 
 
 def main() -> None:
-    # Calls the main script using execv in order to avoid Python import
-    # behaviors.
+    # Subprocess to the main script in order to avoid Python import behaviors.
     this_py = Path(__file__).resolve()
-    actual_py = this_py.parent.parent.parent.joinpath(
-        "bazel", "testing", "lit_autoupdate_base.py"
+    autoupdate_py = this_py.parent.parent.parent.joinpath(
+        "testing", "scripts", "autoupdate_testdata_base.py"
     )
     args = [
-        sys.argv[0],
+        str(autoupdate_py),
         # Flags to configure for explorer testing.
         "--tool=carbon",
         "--autoupdate_arg=dump",
         "--autoupdate_arg=semantics-ir",
         "--testdata=toolchain/semantics/testdata",
     ] + sys.argv[1:]
-    os.execv(actual_py, args)
+    exit(subprocess.call(args))
 
 
 if __name__ == "__main__":