https://github.com/hnrklssn updated https://github.com/llvm/llvm-project/pull/154147
>From b2bdc5bbac4260089097114f7bd5a968c9d6d761 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" <h_ols...@apple.com> Date: Fri, 15 Aug 2025 14:56:11 +0200 Subject: [PATCH 1/4] Reland "[Utils] Add new --update-tests flag to llvm-lit" This reverts commit e495231238b86ae2a3c7bb5f94634c19ca2af19a to reland the --update-tests feature, originally landed in #108425. --- clang/test/lit.cfg.py | 10 ++++++ llvm/docs/CommandGuide/lit.rst | 5 +++ llvm/test/lit.cfg.py | 10 ++++++ llvm/utils/lit/lit/LitConfig.py | 3 ++ llvm/utils/lit/lit/TestRunner.py | 12 +++++++ llvm/utils/lit/lit/cl_arguments.py | 6 ++++ llvm/utils/lit/lit/llvm/config.py | 5 +++ llvm/utils/lit/lit/main.py | 1 + llvm/utils/update_any_test_checks.py | 54 ++++++++++++++++++++++++++-- 9 files changed, 103 insertions(+), 3 deletions(-) diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py index 1957bb1715eb6..12e4d0e454270 100644 --- a/clang/test/lit.cfg.py +++ b/clang/test/lit.cfg.py @@ -410,3 +410,13 @@ def calculate_arch_features(arch_string): # possibly be present in system and user configuration files, so disable # 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) diff --git a/llvm/docs/CommandGuide/lit.rst b/llvm/docs/CommandGuide/lit.rst index eb90e950a3770..15c249d8e6d31 100644 --- a/llvm/docs/CommandGuide/lit.rst +++ b/llvm/docs/CommandGuide/lit.rst @@ -399,6 +399,11 @@ ADDITIONAL OPTIONS Show all features used in the test suite (in ``XFAIL``, ``UNSUPPORTED`` and ``REQUIRES``) 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 ----------- diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py index 8c2d1a454e8f9..bc240425d6d0e 100644 --- a/llvm/test/lit.cfg.py +++ b/llvm/test/lit.cfg.py @@ -715,3 +715,13 @@ def host_unwind_supports_jit(): 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) diff --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py index cb4aef6f72a87..df297b91be1b6 100644 --- a/llvm/utils/lit/lit/LitConfig.py +++ b/llvm/utils/lit/lit/LitConfig.py @@ -39,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 @@ -91,6 +92,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 = [] @property def maxIndividualTestTime(self): diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py index e7cd70766a3dd..f2c5c6d0dbe93 100644 --- a/llvm/utils/lit/lit/TestRunner.py +++ b/llvm/utils/lit/lit/TestRunner.py @@ -1192,6 +1192,18 @@ def executeScriptInternal( str(result.timeoutReached), ) + if litConfig.update_tests: + for test_updater in litConfig.test_updaters: + try: + update_output = test_updater(result, test) + except Exception as e: + out += f"Exception occurred in test updater: {e}" + continue + if update_output: + for line in update_output.splitlines(): + out += f"# {line}\n" + break + return out, err, exitCode, timeoutInfo diff --git a/llvm/utils/lit/lit/cl_arguments.py b/llvm/utils/lit/lit/cl_arguments.py index e88951520e660..8f9211ee3f538 100644 --- a/llvm/utils/lit/lit/cl_arguments.py +++ b/llvm/utils/lit/lit/cl_arguments.py @@ -230,6 +230,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", diff --git a/llvm/utils/lit/lit/llvm/config.py b/llvm/utils/lit/lit/llvm/config.py index 649636d4bcf4c..44119ec8c0eca 100644 --- a/llvm/utils/lit/lit/llvm/config.py +++ b/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") diff --git a/llvm/utils/lit/lit/main.py b/llvm/utils/lit/lit/main.py index 9650a0e901173..5255e2c5e1b51 100755 --- a/llvm/utils/lit/lit/main.py +++ b/llvm/utils/lit/lit/main.py @@ -43,6 +43,7 @@ def main(builtin_params={}): per_test_coverage=opts.per_test_coverage, gtest_sharding=opts.gtest_sharding, maxRetriesPerTest=opts.maxRetriesPerTest, + update_tests=opts.update_tests, ) discovered_tests = lit.discovery.find_tests_for_inputs( diff --git a/llvm/utils/update_any_test_checks.py b/llvm/utils/update_any_test_checks.py index e8eef1a46c504..76fe336593929 100755 --- a/llvm/utils/update_any_test_checks.py +++ b/llvm/utils/update_any_test_checks.py @@ -34,9 +34,12 @@ def find_utc_tool(search_path, utc_name): return None -def run_utc_tool(utc_name, utc_tool, testname): +def run_utc_tool(utc_name, utc_tool, testname, environment): result = subprocess.run( - [utc_tool, testname], stdout=subprocess.PIPE, stderr=subprocess.PIPE + [utc_tool, testname], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=environment, ) return (result.returncode, result.stdout, result.stderr) @@ -60,6 +63,42 @@ def expand_listfile_args(arg_list): return exp_arg_list +def utc_lit_plugin(result, test): + testname = test.getFilePath() + if not testname: + return None + + script_name = os.path.abspath(__file__) + utc_search_path = os.path.join(os.path.dirname(script_name), os.path.pardir) + + with open(testname, "r") as f: + header = f.readline().strip() + + m = RE_ASSERTIONS.search(header) + if m is None: + return None + + utc_name = m.group(1) + utc_tool = find_utc_tool([utc_search_path], utc_name) + if not utc_tool: + return f"update-utc-tests: {utc_name} not found" + + return_code, stdout, stderr = run_utc_tool( + utc_name, utc_tool, testname, test.config.environment + ) + + stderr = stderr.decode(errors="replace") + if return_code != 0: + if stderr: + return f"update-utc-tests: {utc_name} exited with return code {return_code}\n{stderr.rstrip()}" + return f"update-utc-tests: {utc_name} exited with return code {return_code}" + + stdout = stdout.decode(errors="replace") + if stdout: + return f"update-utc-tests: updated {testname}\n{stdout.rstrip()}" + return f"update-utc-tests: updated {testname}" + + def main(): from argparse import RawTextHelpFormatter @@ -78,6 +117,11 @@ def main(): nargs="*", help="Additional directories to scan for update_*_test_checks scripts", ) + parser.add_argument( + "--path", + help="""Additional directories to scan for executables invoked by the update_*_test_checks scripts, +separated by the platform path separator""", + ) parser.add_argument("tests", nargs="+") config = parser.parse_args() @@ -88,6 +132,10 @@ def main(): script_name = os.path.abspath(__file__) utc_search_path.append(os.path.join(os.path.dirname(script_name), os.path.pardir)) + local_env = os.environ.copy() + if config.path: + local_env["PATH"] = config.path + os.pathsep + local_env["PATH"] + not_autogenerated = [] utc_tools = {} have_error = False @@ -117,7 +165,7 @@ def main(): continue future = executor.submit( - run_utc_tool, utc_name, utc_tools[utc_name], testname + run_utc_tool, utc_name, utc_tools[utc_name], testname, local_env ) jobs.append((testname, future)) >From bccb825dbfddcc3bebc0b80d9cd149f5bf6248bb Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" <h_ols...@apple.com> Date: Fri, 15 Aug 2025 17:19:39 +0200 Subject: [PATCH 2/4] [Utils] Adds support for diff based tests to lit's --update-tests This adds an updater to lit's --update-tests flag with support for `diff`. If a RUN line containing the `diff` command fails, this function will use heuristics to try to deduce which file is the "reference" file, and copy the contents of the other file to the reference. If it cannot deduce which file is the reference file, it does nothing. The heuristics are currently: - does one of the files end in .expected while the other does not? Then the .expected file is the reference. - does one of the file paths contain the substring ".tmp" while the other does not? Then the file not containing ".tmp" is the reference. This matches cases where one file path is constructed using the `%t` substitution. --- llvm/utils/lit/lit/DiffUpdater.py | 39 +++++++++++++++++++ llvm/utils/lit/lit/LitConfig.py | 3 +- .../tests/Inputs/diff-test-update/.gitignore | 1 + .../lit/tests/Inputs/diff-test-update/1.in | 1 + .../lit/tests/Inputs/diff-test-update/2.in | 1 + .../Inputs/diff-test-update/diff-bail.test | 3 ++ .../Inputs/diff-test-update/diff-bail2.test | 7 ++++ .../diff-test-update/diff-expected.test | 5 +++ .../Inputs/diff-test-update/diff-tmp-dir.test | 6 +++ .../Inputs/diff-test-update/diff-tmp.test | 3 ++ .../lit/tests/Inputs/diff-test-update/lit.cfg | 8 ++++ llvm/utils/lit/tests/diff-test-update.py | 10 +++++ 12 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 llvm/utils/lit/lit/DiffUpdater.py create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/1.in create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/2.in create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail.test create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail2.test create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/diff-expected.test create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp-dir.test create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp.test create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/lit.cfg create mode 100644 llvm/utils/lit/tests/diff-test-update.py diff --git a/llvm/utils/lit/lit/DiffUpdater.py b/llvm/utils/lit/lit/DiffUpdater.py new file mode 100644 index 0000000000000..2e0a71e1938e3 --- /dev/null +++ b/llvm/utils/lit/lit/DiffUpdater.py @@ -0,0 +1,39 @@ +import shutil + + +def get_source_and_target(a, b): + """ + Try to figure out which file is the test output and which is the reference. + """ + expected_suffix = ".expected" + if a.endswith(expected_suffix) and not b.endswith(expected_suffix): + return b, a + if b.endswith(expected_suffix) and not a.endswith(expected_suffix): + return a, b + + tmp_substr = ".tmp" + if tmp_substr in a and not tmp_substr in b: + return a, b + if tmp_substr in b and not tmp_substr in a: + return b, a + + return None + + +def filter_flags(args): + return [arg for arg in args if not arg.startswith("-")] + + +def diff_test_updater(result, test): + 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) + if not res: + return f"update-diff-test: could not deduce source and target from {a} and {b}" + source, target = res + shutil.copy(source, target) + return f"update-diff-test: copied {source} to {target}" diff --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py index df297b91be1b6..8cef3c1fd8569 100644 --- a/llvm/utils/lit/lit/LitConfig.py +++ b/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): @@ -93,7 +94,7 @@ def __init__( self.per_test_coverage = per_test_coverage self.gtest_sharding = bool(gtest_sharding) self.update_tests = update_tests - self.test_updaters = [] + self.test_updaters = [diff_test_updater] @property def maxIndividualTestTime(self): diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore b/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore new file mode 100644 index 0000000000000..2211df63dd283 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore @@ -0,0 +1 @@ +*.txt diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/1.in b/llvm/utils/lit/tests/Inputs/diff-test-update/1.in new file mode 100644 index 0000000000000..b7d6715e2df11 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/1.in @@ -0,0 +1 @@ +FOO diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/2.in b/llvm/utils/lit/tests/Inputs/diff-test-update/2.in new file mode 100644 index 0000000000000..ba578e48b1836 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/2.in @@ -0,0 +1 @@ +BAR diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail.test b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail.test new file mode 100644 index 0000000000000..ded931384f192 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail.test @@ -0,0 +1,3 @@ +# There is no indication here of which file is the reference file to update +# RUN: diff %S/1.in %S/2.in + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail2.test b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail2.test new file mode 100644 index 0000000000000..26e12a3b2b289 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail2.test @@ -0,0 +1,7 @@ +# RUN: mkdir %t +# RUN: cp %S/1.in %t/1.txt +# RUN: cp %S/2.in %t/2.txt + +# There is no indication here of which file is the reference file to update +# RUN: diff %t/1.txt %t/2.txt + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/diff-expected.test b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-expected.test new file mode 100644 index 0000000000000..a26c6d338f964 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-expected.test @@ -0,0 +1,5 @@ +# RUN: mkdir %t +# RUN: cp %S/1.in %t/my-file.expected +# RUN: cp %S/2.in %t/my-file.txt +# RUN: diff %t/my-file.expected %t/my-file.txt + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp-dir.test b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp-dir.test new file mode 100644 index 0000000000000..929c2c1c6c7d3 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp-dir.test @@ -0,0 +1,6 @@ +# RUN: mkdir %t +# RUN: touch %S/empty.txt +# RUN: cp %S/1.in %t/1.txt + +# RUN: diff %t/1.txt %S/empty.txt + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp.test b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp.test new file mode 100644 index 0000000000000..042bf244ebaa1 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp.test @@ -0,0 +1,3 @@ +# RUN: cp %S/1.in %t.txt +# RUN: cp %S/2.in %S/diff-t-out.txt +# RUN: diff %t.txt %S/diff-t-out.txt diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/lit.cfg b/llvm/utils/lit/tests/Inputs/diff-test-update/lit.cfg new file mode 100644 index 0000000000000..9bd255276638a --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/lit.cfg @@ -0,0 +1,8 @@ +import lit.formats + +config.name = "diff-test-update" +config.suffixes = [".test"] +config.test_format = lit.formats.ShTest() +config.test_source_root = None +config.test_exec_root = None + diff --git a/llvm/utils/lit/tests/diff-test-update.py b/llvm/utils/lit/tests/diff-test-update.py new file mode 100644 index 0000000000000..21b869d120655 --- /dev/null +++ b/llvm/utils/lit/tests/diff-test-update.py @@ -0,0 +1,10 @@ +# RUN: not %{lit} --update-tests -v %S/Inputs/diff-test-update | FileCheck %s + +# CHECK: # update-diff-test: could not deduce source and target from {{.*}}/Inputs/diff-test-update/1.in and {{.*}}/Inputs/diff-test-update/2.in +# CHECK: # update-diff-test: could not deduce source and target from {{.*}}/diff-test-update/Output/diff-bail2.test.tmp/1.txt and {{.*}}/diff-test-update/Output/diff-bail2.test.tmp/2.txt +# CHECK: # update-diff-test: copied {{.*}}/Output/diff-expected.test.tmp/my-file.txt to {{.*}}/Output/diff-expected.test.tmp/my-file.expected +# CHECK: # update-diff-test: copied {{.*}}/Output/diff-tmp-dir.test.tmp/1.txt to {{.*}}/Inputs/diff-test-update/empty.txt +# CHECK: # update-diff-test: copied {{.*}}/Inputs/diff-test-update/Output/diff-tmp.test.tmp.txt to {{.*}}/Inputs/diff-test-update/diff-t-out.txt + + +# CHECK: Failed: 5 (100.00%) >From 64b9237e63d8255a154d0921d3b8367e730cfe66 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" <h_ols...@apple.com> Date: Mon, 18 Aug 2025 09:43:09 -0700 Subject: [PATCH 3/4] [Utils] Document DiffUpdater.py --- llvm/utils/lit/lit/DiffUpdater.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/llvm/utils/lit/lit/DiffUpdater.py b/llvm/utils/lit/lit/DiffUpdater.py index 2e0a71e1938e3..67a3c04b69e1b 100644 --- a/llvm/utils/lit/lit/DiffUpdater.py +++ b/llvm/utils/lit/lit/DiffUpdater.py @@ -1,5 +1,20 @@ import shutil +""" +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 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. +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. + 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. +""" def get_source_and_target(a, b): """ >From aa45e2605a3bd52efe8df73f7b9d4d52e1d60dc0 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" <h_ols...@apple.com> Date: Mon, 18 Aug 2025 09:54:43 -0700 Subject: [PATCH 4/4] Add whitespace --- llvm/utils/lit/lit/DiffUpdater.py | 1 + 1 file changed, 1 insertion(+) diff --git a/llvm/utils/lit/lit/DiffUpdater.py b/llvm/utils/lit/lit/DiffUpdater.py index 67a3c04b69e1b..de0001a94f0ba 100644 --- a/llvm/utils/lit/lit/DiffUpdater.py +++ b/llvm/utils/lit/lit/DiffUpdater.py @@ -16,6 +16,7 @@ - Only update the parts that need updating (based on the diff output). Could help avoid noisy updates when e.g. whitespace changes are ignored. """ + def get_source_and_target(a, b): """ Try to figure out which file is the test output and which is the reference. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits