Merge pull request #11400 from hnrklssn/cherry-pick-update-tests-stable

[Cherry-pick][stable/20240723] Add --update-tests to llvm-lit

Author: hnrklssn

clang/test/lit.cfg.py

@@ -373,8 +373,19 @@ def calculate_arch_features(arch_string):
 # default configs for the test runs.
 config.environment["CLANG_NO_DEFAULT_CONFIG"] = "1"
 
+if lit_config.update_tests:
+    import sys
+    import os
+
+    utilspath = os.path.join(config.llvm_src_root, "utils")
+    sys.path.append(utilspath)
+    from update_any_test_checks import utc_lit_plugin
+
+    lit_config.test_updaters.append(utc_lit_plugin)
+
 # Restrict the size of the on-disk CAS for tests. This allows testing in
 # constrained environments (e.g. small TMPDIR). It also prevents leaving
 # behind large files on file systems that do not support sparse files if a test
 # crashes before resizing the file.
 config.environment["LLVM_CAS_MAX_MAPPING_SIZE"] = "%d" % (100 * 1024 * 1024)
+

llvm/docs/CommandGuide/lit.rst

@@ -313,6 +313,11 @@ ADDITIONAL OPTIONS
 
  List all of the discovered tests and exit.
 
+.. option:: --update-tests
+
+ Pass failing tests to functions in the ``lit_config.test_updaters`` list to
+ check whether any of them know how to update the test to make it pass.
+
 EXIT STATUS
 -----------
 

llvm/test/lit.cfg.py

@@ -632,3 +632,13 @@ def have_ld64_plugin_support():
 
 if config.has_logf128:
     config.available_features.add("has_logf128")
+
+if lit_config.update_tests:
+    import sys
+    import os
+
+    utilspath = os.path.join(config.llvm_src_root, "utils")
+    sys.path.append(utilspath)
+    from update_any_test_checks import utc_lit_plugin
+
+    lit_config.test_updaters.append(utc_lit_plugin)

llvm/utils/lit/lit/DiffUpdater.py

@@ -0,0 +1,161 @@
+import shutil
+import os
+import shlex
+import pathlib
+
+"""
+This file provides the `diff_test_updater` function, which is invoked on failed RUN lines when lit is executed with --update-tests.
+It checks whether the failed command is `diff` and, if so, uses heuristics to determine which file is the checked-in reference file and which file is output from the test case.
+The heuristics are currently as follows:
+    - if exactly one file originates from the `split-file` command, that file is the reference file and the other is the output file
+    - if exactly one file ends with ".expected" (common pattern in LLVM), that file is the reference file and the other is the output file
+    - if exactly one file path contains ".tmp" (e.g. because it contains the expansion of "%t"), that file is the reference file and the other is the output file
+If the command matches one of these patterns the output file content is copied to the reference file to make the test pass.
+If the reference file originated in `split-file`, the output file content is instead copied to the corresponding slice of the test file.
+Otherwise the test is ignored.
+
+Possible improvements:
+    - Support stdin patterns like "my_binary %s | diff expected.txt"
+    - Scan RUN lines to see if a file is the source of output from a previous command (other than `split-file`).
+      If it is then it is not a reference file that can be copied to, regardless of name, since the test will overwrite it anyways.
+    - Only update the parts that need updating (based on the diff output). Could help avoid noisy updates when e.g. whitespace changes are ignored.
+"""
+
+
+class NormalFileTarget:
+    def __init__(self, target):
+        self.target = target
+
+    def copyFrom(self, source):
+        shutil.copy(source, self.target)
+
+    def __str__(self):
+        return self.target
+
+
+class SplitFileTarget:
+    def __init__(self, slice_start_idx, test_path, lines):
+        self.slice_start_idx = slice_start_idx
+        self.test_path = test_path
+        self.lines = lines
+
+    def copyFrom(self, source):
+        lines_before = self.lines[: self.slice_start_idx + 1]
+        self.lines = self.lines[self.slice_start_idx + 1 :]
+        slice_end_idx = None
+        for i, l in enumerate(self.lines):
+            if SplitFileTarget._get_split_line_path(l) != None:
+                slice_end_idx = i
+                break
+        if slice_end_idx is not None:
+            lines_after = self.lines[slice_end_idx:]
+        else:
+            lines_after = []
+        with open(source, "r") as f:
+            new_lines = lines_before + f.readlines() + lines_after
+        with open(self.test_path, "w") as f:
+            for l in new_lines:
+                f.write(l)
+
+    def __str__(self):
+        return f"slice in {self.test_path}"
+
+    @staticmethod
+    def get_target_dir(commands, test_path):
+        # posix=True breaks Windows paths because \ is treated as an escaping character
+        for cmd in commands:
+            split = shlex.split(cmd, posix=False)
+            if "split-file" not in split:
+                continue
+            start_idx = split.index("split-file")
+            split = split[start_idx:]
+            if len(split) < 3:
+                continue
+            p = unquote(split[1].strip())
+            if not test_path.samefile(p):
+                continue
+            return unquote(split[2].strip())
+        return None
+
+    @staticmethod
+    def create(path, commands, test_path, target_dir):
+        path = pathlib.Path(path)
+        with open(test_path, "r") as f:
+            lines = f.readlines()
+        for i, l in enumerate(lines):
+            p = SplitFileTarget._get_split_line_path(l)
+            if p and path.samefile(os.path.join(target_dir, p)):
+                idx = i
+                break
+        else:
+            return None
+        return SplitFileTarget(idx, test_path, lines)
+
+    @staticmethod
+    def _get_split_line_path(l):
+        if len(l) < 6:
+            return None
+        if l.startswith("//"):
+            l = l[2:]
+        else:
+            l = l[1:]
+        if l.startswith("--- "):
+            l = l[4:]
+        else:
+            return None
+        return l.rstrip()
+
+
+def unquote(s):
+    if len(s) > 1 and s[0] == s[-1] and (s[0] == '"' or s[0] == "'"):
+        return s[1:-1]
+    return s
+
+
+def get_source_and_target(a, b, test_path, commands):
+    """
+    Try to figure out which file is the test output and which is the reference.
+    """
+    split_target_dir = SplitFileTarget.get_target_dir(commands, test_path)
+    if split_target_dir:
+        a_target = SplitFileTarget.create(a, commands, test_path, split_target_dir)
+        b_target = SplitFileTarget.create(b, commands, test_path, split_target_dir)
+        if a_target and b_target:
+            return None
+        if a_target:
+            return b, a_target
+        if b_target:
+            return a, b_target
+
+    expected_suffix = ".expected"
+    if a.endswith(expected_suffix) and not b.endswith(expected_suffix):
+        return b, NormalFileTarget(a)
+    if b.endswith(expected_suffix) and not a.endswith(expected_suffix):
+        return a, NormalFileTarget(b)
+
+    tmp_substr = ".tmp"
+    if tmp_substr in a and not tmp_substr in b:
+        return a, NormalFileTarget(b)
+    if tmp_substr in b and not tmp_substr in a:
+        return b, NormalFileTarget(a)
+
+    return None
+
+
+def filter_flags(args):
+    return [arg for arg in args if not arg.startswith("-")]
+
+
+def diff_test_updater(result, test, commands):
+    args = filter_flags(result.command.args)
+    if len(args) != 3:
+        return None
+    [cmd, a, b] = args
+    if cmd != "diff":
+        return None
+    res = get_source_and_target(a, b, pathlib.Path(test.getFilePath()), commands)
+    if not res:
+        return f"update-diff-test: could not deduce source and target from {a} and {b}"
+    source, target = res
+    target.copyFrom(source)
+    return f"update-diff-test: copied {source} to {target}"

llvm/utils/lit/lit/LitConfig.py

@@ -8,6 +8,7 @@
 import lit.formats
 import lit.TestingConfig
 import lit.util
+from lit.DiffUpdater import diff_test_updater
 
 # LitConfig must be a new style class for properties to work
 class LitConfig(object):
@@ -38,6 +39,7 @@ def __init__(
         parallelism_groups={},
         per_test_coverage=False,
         gtest_sharding=True,
+        update_tests=False,
     ):
         # The name of the test runner.
         self.progname = progname
@@ -89,6 +91,8 @@ def __init__(
         self.parallelism_groups = parallelism_groups
         self.per_test_coverage = per_test_coverage
         self.gtest_sharding = bool(gtest_sharding)
+        self.update_tests = update_tests
+        self.test_updaters = [diff_test_updater]
 
     @property
     def maxIndividualTestTime(self):

llvm/utils/lit/lit/TestRunner.py

@@ -13,6 +13,7 @@
 import tempfile
 import threading
 import typing
+import traceback
 from typing import Optional, Tuple
 
 import io
@@ -47,6 +48,16 @@ def __init__(self, message):
         super().__init__(message)
 
 
+class TestUpdaterException(Exception):
+    """
+    There was an error not during test execution, but while invoking a function
+    in test_updaters on a failing RUN line.
+    """
+
+    def __init__(self, message):
+        super().__init__(message)
+
+
 kIsWindows = platform.system() == "Windows"
 
 # Don't use close_fds on Windows.
@@ -1154,6 +1165,28 @@ def executeScriptInternal(
                 str(result.timeoutReached),
             )
 
+        if (
+            litConfig.update_tests
+            and result.exitCode != 0
+            and not timeoutInfo
+            # In theory tests marked XFAIL can fail in the form of XPASS, but the
+            # test updaters are not expected to be able to fix that, so always skip for XFAIL
+            and not test.isExpectedToFail()
+        ):
+            for test_updater in litConfig.test_updaters:
+                try:
+                    update_output = test_updater(result, test, commands)
+                except Exception as e:
+                    output = out
+                    output += err
+                    output += "Exception occurred in test updater:\n"
+                    output += traceback.format_exc()
+                    raise TestUpdaterException(output)
+                if update_output:
+                    for line in update_output.splitlines():
+                        out += f"# {line}\n"
+                    break
+
     return out, err, exitCode, timeoutInfo
 
 

llvm/utils/lit/lit/cl_arguments.py

@@ -204,6 +204,12 @@ def parse_args():
         action="store_true",
         help="Exit with status zero even if some tests fail",
     )
+    execution_group.add_argument(
+        "--update-tests",
+        dest="update_tests",
+        action="store_true",
+        help="Try to update regression tests to reflect current behavior, if possible",
+    )
     execution_test_time_group = execution_group.add_mutually_exclusive_group()
     execution_test_time_group.add_argument(
         "--skip-test-time-recording",

llvm/utils/lit/lit/llvm/config.py

@@ -64,12 +64,17 @@ def __init__(self, lit_config, config):
             self.with_environment("_TAG_REDIR_ERR", "TXT")
             self.with_environment("_CEE_RUNOPTS", "FILETAG(AUTOCVT,AUTOTAG) POSIX(ON)")
 
+        if lit_config.update_tests:
+            self.use_lit_shell = True
+
         # Choose between lit's internal shell pipeline runner and a real shell.
         # If LIT_USE_INTERNAL_SHELL is in the environment, we use that as an
         # override.
         lit_shell_env = os.environ.get("LIT_USE_INTERNAL_SHELL")
         if lit_shell_env:
             self.use_lit_shell = lit.util.pythonize_bool(lit_shell_env)
+            if not self.use_lit_shell and lit_config.update_tests:
+                print("note: --update-tests is not supported when using external shell")
 
         if not self.use_lit_shell:
             features.add("shell")

llvm/utils/lit/lit/main.py

@@ -42,6 +42,7 @@ def main(builtin_params={}):
         config_prefix=opts.configPrefix,
         per_test_coverage=opts.per_test_coverage,
         gtest_sharding=opts.gtest_sharding,
+        update_tests=opts.update_tests,
     )
 
     discovered_tests = lit.discovery.find_tests_for_inputs(

llvm/utils/lit/lit/worker.py

@@ -13,6 +13,7 @@
 
 import lit.Test
 import lit.util
+from lit.TestRunner import TestUpdaterException
 
 
 _lit_config = None
@@ -75,6 +76,10 @@ def _execute_test_handle_errors(test, lit_config):
     try:
         result = test.config.test_format.execute(test, lit_config)
         return _adapt_result(result)
+    except TestUpdaterException as e:
+        if lit_config.debug:
+            raise
+        return lit.Test.Result(lit.Test.UNRESOLVED, str(e))
     except:
         if lit_config.debug:
             raise