From ea6d63a7f02dc5e07e3a21c07f399c52dd685feb Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Mon, 18 Aug 2025 20:24:27 -0700 Subject: [PATCH 01/10] Reland "[Utils] Add new --update-tests flag to llvm-lit" (#153821) This reverts commit https://github.com/llvm/llvm-project/commit/e495231238b86ae2a3c7bb5f94634c19ca2af19a to reland the --update-tests feature, originally landed in https://github.com/llvm/llvm-project/pull/108425. (cherry picked from commit e1ff432eb66b05299d1cec842abf17c06eaa9883) --- clang/test/lit.cfg.py | 11 ++++++ 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, 104 insertions(+), 3 deletions(-) diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py index 6b508e372267d..dbc63b4f0f97a 100644 --- a/clang/test/lit.cfg.py +++ b/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) + diff --git a/llvm/docs/CommandGuide/lit.rst b/llvm/docs/CommandGuide/lit.rst index c9d5baba3e2f4..adf0075215c46 100644 --- a/llvm/docs/CommandGuide/lit.rst +++ b/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 ----------- diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py index 3b1e6b72fdde9..588deccfd5a6c 100644 --- a/llvm/test/lit.cfg.py +++ b/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) diff --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py index 5dc712ae28370..198a2bf317233 100644 --- a/llvm/utils/lit/lit/LitConfig.py +++ b/llvm/utils/lit/lit/LitConfig.py @@ -38,6 +38,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 +90,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 da7fa86fd3917..b4cabbca37aa4 100644 --- a/llvm/utils/lit/lit/TestRunner.py +++ b/llvm/utils/lit/lit/TestRunner.py @@ -1154,6 +1154,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 ed78256ee414b..dcbe553c6d482 100644 --- a/llvm/utils/lit/lit/cl_arguments.py +++ b/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", diff --git a/llvm/utils/lit/lit/llvm/config.py b/llvm/utils/lit/lit/llvm/config.py index b3dfddba483f5..4ef24ae20f8c1 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 24ba804f0c363..745e376de7d52 100755 --- a/llvm/utils/lit/lit/main.py +++ b/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( 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 eb6557c96164c35b772d6ca77d89178154495a4d Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Mon, 25 Aug 2025 13:29:10 -0700 Subject: [PATCH 02/10] [Util] Only run --update-tests functions on failing tests (#155148) The early exit we relied on to only invoke test updaters for failing tests requires that there was no output to stdout or stderr, and that timeouts weren't enabled. When these conditions weren't fulfilled, test updaters would be invoked even on passing or XFAILed tests. (cherry picked from commit 58996c0ba62db2f4f704e7d583306e54e06b5953) --- llvm/utils/lit/lit/TestRunner.py | 27 +++++++++++-- llvm/utils/lit/lit/worker.py | 5 +++ .../tests/Inputs/pass-test-update/fail.test | 1 + .../lit/tests/Inputs/pass-test-update/lit.cfg | 14 +++++++ .../Inputs/pass-test-update/pass-silent.test | 2 + .../tests/Inputs/pass-test-update/pass.test | 1 + .../Inputs/pass-test-update/should_not_run.py | 2 + .../tests/Inputs/pass-test-update/xfail.test | 2 + .../tests/Inputs/pass-test-update/xpass.test | 2 + llvm/utils/lit/tests/pass-test-update.py | 39 +++++++++++++++++++ 10 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/fail.test create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/lit.cfg create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/pass-silent.test create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/pass.test create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/xfail.test create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/xpass.test create mode 100644 llvm/utils/lit/tests/pass-test-update.py diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py index b4cabbca37aa4..b296f83bfbfa1 100644 --- a/llvm/utils/lit/lit/TestRunner.py +++ b/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,13 +1165,23 @@ def executeScriptInternal( str(result.timeoutReached), ) - if litConfig.update_tests: + 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) except Exception as e: - out += f"Exception occurred in test updater: {e}" - continue + 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" diff --git a/llvm/utils/lit/lit/worker.py b/llvm/utils/lit/lit/worker.py index 8e78bfd45d38b..0e36f3e9a2424 100644 --- a/llvm/utils/lit/lit/worker.py +++ b/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 diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/fail.test b/llvm/utils/lit/tests/Inputs/pass-test-update/fail.test new file mode 100644 index 0000000000000..a410ef9293129 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/pass-test-update/fail.test @@ -0,0 +1 @@ +# RUN: not echo "fail" diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/lit.cfg b/llvm/utils/lit/tests/Inputs/pass-test-update/lit.cfg new file mode 100644 index 0000000000000..e0dbc3220678c --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/pass-test-update/lit.cfg @@ -0,0 +1,14 @@ +import lit.formats + +config.name = "pass-test-update" +config.suffixes = [".test"] +config.test_format = lit.formats.ShTest() +config.test_source_root = None +config.test_exec_root = None + +import sys +import os +sys.path.append(os.path.dirname(__file__)) +from should_not_run import should_not_run +lit_config.test_updaters.append(should_not_run) + diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/pass-silent.test b/llvm/utils/lit/tests/Inputs/pass-test-update/pass-silent.test new file mode 100644 index 0000000000000..07cafbc66a897 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/pass-test-update/pass-silent.test @@ -0,0 +1,2 @@ +# RUN: echo "" + diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/pass.test b/llvm/utils/lit/tests/Inputs/pass-test-update/pass.test new file mode 100644 index 0000000000000..3a12c5fee4bb0 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/pass-test-update/pass.test @@ -0,0 +1 @@ +# RUN: echo "passed" diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py b/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py new file mode 100644 index 0000000000000..0fda62c832f08 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py @@ -0,0 +1,2 @@ +def should_not_run(foo, bar): + raise Exception("this test updater should only run on failure") diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/xfail.test b/llvm/utils/lit/tests/Inputs/pass-test-update/xfail.test new file mode 100644 index 0000000000000..044b6a19b4a19 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/pass-test-update/xfail.test @@ -0,0 +1,2 @@ +# XFAIL: * +# RUN: not echo "failed" diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/xpass.test b/llvm/utils/lit/tests/Inputs/pass-test-update/xpass.test new file mode 100644 index 0000000000000..e0c1c39762ee9 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/pass-test-update/xpass.test @@ -0,0 +1,2 @@ +# XFAIL: * +# RUN: echo "accidentally passed" diff --git a/llvm/utils/lit/tests/pass-test-update.py b/llvm/utils/lit/tests/pass-test-update.py new file mode 100644 index 0000000000000..e3efb9df919c4 --- /dev/null +++ b/llvm/utils/lit/tests/pass-test-update.py @@ -0,0 +1,39 @@ +# RUN: %{lit} --update-tests --ignore-fail -v %S/Inputs/pass-test-update | FileCheck %s --implicit-check-not Exception + +# CHECK: UNRESOLVED: pass-test-update :: fail.test (1 of 5) +# CHECK-NEXT: ******************** TEST 'pass-test-update :: fail.test' FAILED ******************** +# CHECK-NEXT: # {{R}}UN: at line 1 +# CHECK-NEXT: not echo "fail" +# CHECK-NEXT: # executed command: not echo fail +# CHECK-NEXT: # .---command stdout------------ +# CHECK-NEXT: # | fail +# CHECK-NEXT: # `----------------------------- +# CHECK-NEXT: # error: command failed with exit status: 1 +# CHECK-NEXT: Exception occurred in test updater: +# CHECK-NEXT: Traceback (most recent call last): +# CHECK-NEXT: File {{.*}}, line {{.*}}, in {{.*}} +# CHECK-NEXT: update_output = test_updater(result, test) +# CHECK-NEXT: File "{{.*}}/should_not_run.py", line {{.*}}, in should_not_run +# CHECK-NEXT: raise Exception("this test updater should only run on failure") +# CHECK-NEXT: Exception: this test updater should only run on failure +# CHECK-EMPTY: +# CHECK-NEXT: ******************** +# CHECK-NEXT: PASS: pass-test-update :: pass-silent.test (2 of 5) +# CHECK-NEXT: PASS: pass-test-update :: pass.test (3 of 5) +# CHECK-NEXT: {{X}}FAIL: pass-test-update :: xfail.test (4 of 5) +# CHECK-NEXT: XPASS: pass-test-update :: xpass.test (5 of 5) +# CHECK-NEXT: ******************** TEST 'pass-test-update :: xpass.test' FAILED ******************** +# CHECK-NEXT: Exit Code: 0 +# CHECK-EMPTY: +# CHECK-NEXT: Command Output (stdout): +# CHECK-NEXT: -- +# CHECK-NEXT: # {{R}}UN: at line 2 +# CHECK-NEXT: echo "accidentally passed" +# CHECK-NEXT: # executed command: echo 'accidentally passed' +# CHECK-NEXT: # .---command stdout------------ +# CHECK-NEXT: # | accidentally passed +# CHECK-NEXT: # `----------------------------- +# CHECK-EMPTY: +# CHECK-NEXT: -- +# CHECK-EMPTY: +# CHECK-NEXT: ******************** From eabe978931536ee7a6310f6c692c136442e00170 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Mon, 25 Aug 2025 15:03:06 -0700 Subject: [PATCH 03/10] [Util] Make pass-test-update.py test case more resilient (NFC) (#155303) This test case matches against python traceback output, which seems to vary slightly between versions. This relaxes the constraints a bit to make the test pass on buildbots. (cherry picked from commit 31948b3a46e6a2d02fb058351c7101eda21e84e2) --- llvm/utils/lit/tests/pass-test-update.py | 69 +++++++++++------------- 1 file changed, 32 insertions(+), 37 deletions(-) diff --git a/llvm/utils/lit/tests/pass-test-update.py b/llvm/utils/lit/tests/pass-test-update.py index e3efb9df919c4..f23bce9838f35 100644 --- a/llvm/utils/lit/tests/pass-test-update.py +++ b/llvm/utils/lit/tests/pass-test-update.py @@ -1,39 +1,34 @@ # RUN: %{lit} --update-tests --ignore-fail -v %S/Inputs/pass-test-update | FileCheck %s --implicit-check-not Exception -# CHECK: UNRESOLVED: pass-test-update :: fail.test (1 of 5) -# CHECK-NEXT: ******************** TEST 'pass-test-update :: fail.test' FAILED ******************** -# CHECK-NEXT: # {{R}}UN: at line 1 -# CHECK-NEXT: not echo "fail" -# CHECK-NEXT: # executed command: not echo fail -# CHECK-NEXT: # .---command stdout------------ -# CHECK-NEXT: # | fail -# CHECK-NEXT: # `----------------------------- -# CHECK-NEXT: # error: command failed with exit status: 1 -# CHECK-NEXT: Exception occurred in test updater: -# CHECK-NEXT: Traceback (most recent call last): -# CHECK-NEXT: File {{.*}}, line {{.*}}, in {{.*}} -# CHECK-NEXT: update_output = test_updater(result, test) -# CHECK-NEXT: File "{{.*}}/should_not_run.py", line {{.*}}, in should_not_run -# CHECK-NEXT: raise Exception("this test updater should only run on failure") -# CHECK-NEXT: Exception: this test updater should only run on failure -# CHECK-EMPTY: -# CHECK-NEXT: ******************** -# CHECK-NEXT: PASS: pass-test-update :: pass-silent.test (2 of 5) -# CHECK-NEXT: PASS: pass-test-update :: pass.test (3 of 5) -# CHECK-NEXT: {{X}}FAIL: pass-test-update :: xfail.test (4 of 5) -# CHECK-NEXT: XPASS: pass-test-update :: xpass.test (5 of 5) -# CHECK-NEXT: ******************** TEST 'pass-test-update :: xpass.test' FAILED ******************** -# CHECK-NEXT: Exit Code: 0 -# CHECK-EMPTY: -# CHECK-NEXT: Command Output (stdout): -# CHECK-NEXT: -- -# CHECK-NEXT: # {{R}}UN: at line 2 -# CHECK-NEXT: echo "accidentally passed" -# CHECK-NEXT: # executed command: echo 'accidentally passed' -# CHECK-NEXT: # .---command stdout------------ -# CHECK-NEXT: # | accidentally passed -# CHECK-NEXT: # `----------------------------- -# CHECK-EMPTY: -# CHECK-NEXT: -- -# CHECK-EMPTY: -# CHECK-NEXT: ******************** +# CHECK: UNRESOLVED: pass-test-update :: fail.test (1 of 5) +# CHECK: ******************** TEST 'pass-test-update :: fail.test' FAILED ******************** +# CHECK: # {{R}}UN: at line 1 +# CHECK: not echo "fail" +# CHECK: # executed command: not echo fail +# CHECK: # .---command stdout------------ +# CHECK: # | fail +# CHECK: # `----------------------------- +# CHECK: # error: command failed with exit status: 1 +# CHECK: Exception occurred in test updater: +# CHECK: Traceback (most recent call last): +# CHECK: File {{.*}}, line {{.*}}, in {{.*}} +# CHECK: update_output = test_updater(result, test) +# CHECK: File "{{.*}}/should_not_run.py", line {{.*}}, in should_not_run +# CHECK: raise Exception("this test updater should only run on failure") +# CHECK: Exception: this test updater should only run on failure +# CHECK: ******************** +# CHECK: PASS: pass-test-update :: pass-silent.test (2 of 5) +# CHECK: PASS: pass-test-update :: pass.test (3 of 5) +# CHECK: {{X}}FAIL: pass-test-update :: xfail.test (4 of 5) +# CHECK: XPASS: pass-test-update :: xpass.test (5 of 5) +# CHECK: ******************** TEST 'pass-test-update :: xpass.test' FAILED ******************** +# CHECK: Exit Code: 0 +# CHECK: Command Output (stdout): +# CHECK: -- +# CHECK: # {{R}}UN: at line 2 +# CHECK: echo "accidentally passed" +# CHECK: # executed command: echo 'accidentally passed' +# CHECK: # .---command stdout------------ +# CHECK: # | accidentally passed +# CHECK: # `----------------------------- +# CHECK: ******************** From 3324de9e6425d97a00332f473de6890182347b86 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Mon, 8 Sep 2025 15:46:48 -0700 Subject: [PATCH 04/10] [Utils] Adds support for diff based tests to lit's --update-tests (#154147) 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. (cherry picked from commit 6c3f18ebcaf234a842b8d169425ad73b93445d68) --- llvm/utils/lit/lit/DiffUpdater.py | 55 +++++++++++++++++++ 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, 102 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..de0001a94f0ba --- /dev/null +++ b/llvm/utils/lit/lit/DiffUpdater.py @@ -0,0 +1,55 @@ +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): + """ + 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 198a2bf317233..dffafeb8b6081 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): @@ -91,7 +92,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 5f897fd42ace8840b2030333d5ff47ab5b206360 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Mon, 8 Sep 2025 17:12:27 -0700 Subject: [PATCH 05/10] [Utils] Ensure that empty.txt is always empty (#157576) Previously this test case would `touch %S/empty.txt` to create and empty file. The test case then copies contents to that file, so if run a second time the `touch` command would not create an empty file. `llvm/utils/lit/tests/diff-test-update.py` also no longer matches against file paths other than the final bit of the file name. This prevents test failures on Windows due to different path separators. (cherry picked from commit 82ef4ee725a459c137dd1f5419cf26deb15a14c8) --- .../utils/lit/tests/Inputs/diff-test-update/.gitignore | 3 ++- .../tests/Inputs/diff-test-update/diff-tmp-dir.test | 4 +++- llvm/utils/lit/tests/diff-test-update.py | 10 +++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore b/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore index 2211df63dd283..dd373bf9e0c66 100644 --- a/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore @@ -1 +1,2 @@ -*.txt +; diff-tmp-dir.test clobbers this file +empty.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 index 929c2c1c6c7d3..a4fab7e74c007 100644 --- 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 @@ -1,5 +1,7 @@ # RUN: mkdir %t -# RUN: touch %S/empty.txt +# Tests that if file A is in the %t directory and file B is not, +# the diff test updater copies from file A to B. +# RUN: echo "" > %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/diff-test-update.py b/llvm/utils/lit/tests/diff-test-update.py index 21b869d120655..c37d0dccc727c 100644 --- a/llvm/utils/lit/tests/diff-test-update.py +++ b/llvm/utils/lit/tests/diff-test-update.py @@ -1,10 +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: # update-diff-test: could not deduce source and target from {{.*}}1.in and {{.*}}2.in +# CHECK: # update-diff-test: could not deduce source and target from {{.*}}1.txt and {{.*}}2.txt +# CHECK: # update-diff-test: copied {{.*}}my-file.txt to {{.*}}my-file.expected +# CHECK: # update-diff-test: copied {{.*}}1.txt to {{.*}}empty.txt +# CHECK: # update-diff-test: copied {{.*}}diff-tmp.test.tmp.txt to {{.*}}diff-t-out.txt # CHECK: Failed: 5 (100.00%) From 54e1d40f4fb3c0fd825cce8d3ce70f9bf2757d2a Mon Sep 17 00:00:00 2001 From: dyung Date: Tue, 26 Aug 2025 01:15:15 -0400 Subject: [PATCH 06/10] Fix test added in #155148 work with Windows style path separators. (#155354) Should fix Windows build bot failures such as https://lab.llvm.org/buildbot/#/builders/46/builds/22281. The test (and the followup fix in #155303) did not properly account for Windows style path separators. (cherry picked from commit 7d35e29d7a7f89c828e91c03d3827a43deaa5dec) --- llvm/utils/lit/tests/pass-test-update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/utils/lit/tests/pass-test-update.py b/llvm/utils/lit/tests/pass-test-update.py index f23bce9838f35..00a4025be660e 100644 --- a/llvm/utils/lit/tests/pass-test-update.py +++ b/llvm/utils/lit/tests/pass-test-update.py @@ -13,7 +13,7 @@ # CHECK: Traceback (most recent call last): # CHECK: File {{.*}}, line {{.*}}, in {{.*}} # CHECK: update_output = test_updater(result, test) -# CHECK: File "{{.*}}/should_not_run.py", line {{.*}}, in should_not_run +# CHECK: File "{{.*}}{{/|\\}}should_not_run.py", line {{.*}}, in should_not_run # CHECK: raise Exception("this test updater should only run on failure") # CHECK: Exception: this test updater should only run on failure # CHECK: ******************** From 6933689dd7ecc00427d33ddd2dfdf10d7d081756 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Thu, 11 Sep 2025 12:58:59 -0700 Subject: [PATCH 07/10] [Utils] Add support for split-file to diff_test_updater (#157765) (cherry picked from commit 9eb17cc0343d09264ea875038901d1c6541dcef7) --- llvm/utils/lit/lit/DiffUpdater.py | 117 ++++++++++++++++-- llvm/utils/lit/lit/TestRunner.py | 2 +- .../tests/Inputs/diff-test-update/.gitignore | 8 ++ .../multiple-split-file-populated.in | 17 +++ .../diff-test-update/multiple-split-file.in | 13 ++ .../diff-test-update/multiple-split-file.out | 14 +++ .../single-split-file-no-expected.in | 6 + .../single-split-file-no-expected.out | 6 + .../single-split-file-populated.in | 7 ++ .../diff-test-update/single-split-file.in | 5 + .../diff-test-update/single-split-file.out | 6 + .../Inputs/diff-test-update/split-both.test | 11 ++ .../diff-test-update/split-c-comments.in | 6 + .../diff-test-update/split-c-comments.out | 6 + .../diff-test-update/split-whitespace.in | 6 + .../diff-test-update/split-whitespace.out | 6 + .../diff-test-update/unrelated-split.test | 11 ++ .../Inputs/pass-test-update/should_not_run.py | 2 +- llvm/utils/lit/tests/diff-test-update.py | 21 +++- llvm/utils/lit/tests/pass-test-update.py | 2 +- llvm/utils/update_any_test_checks.py | 2 +- 21 files changed, 260 insertions(+), 14 deletions(-) create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file-populated.in create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file.in create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file.out create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-no-expected.in create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-no-expected.out create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-populated.in create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file.in create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file.out create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/split-both.test create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/split-c-comments.in create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/split-c-comments.out create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/split-whitespace.in create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/split-whitespace.out create mode 100644 llvm/utils/lit/tests/Inputs/diff-test-update/unrelated-split.test diff --git a/llvm/utils/lit/lit/DiffUpdater.py b/llvm/utils/lit/lit/DiffUpdater.py index de0001a94f0ba..5bba2d70991df 100644 --- a/llvm/utils/lit/lit/DiffUpdater.py +++ b/llvm/utils/lit/lit/DiffUpdater.py @@ -1,37 +1,136 @@ import shutil +import os +import shlex """ 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. + - 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. """ -def get_source_and_target(a, b): +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): + for cmd in commands: + split = shlex.split(cmd) + if "split-file" not in split: + continue + start_idx = split.index("split-file") + split = split[start_idx:] + if len(split) < 3: + continue + if split[1].strip() != test_path: + continue + return split[2].strip() + return None + + @staticmethod + def create(path, commands, test_path, target_dir): + filename = path.replace(target_dir, "") + if filename.startswith(os.sep): + filename = filename[len(os.sep) :] + 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 == filename: + 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 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, a + return b, NormalFileTarget(a) if b.endswith(expected_suffix) and not a.endswith(expected_suffix): - return a, b + return a, NormalFileTarget(b) tmp_substr = ".tmp" if tmp_substr in a and not tmp_substr in b: - return a, b + return a, NormalFileTarget(b) if tmp_substr in b and not tmp_substr in a: - return b, a + return b, NormalFileTarget(a) return None @@ -40,16 +139,16 @@ def filter_flags(args): return [arg for arg in args if not arg.startswith("-")] -def diff_test_updater(result, test): +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) + res = get_source_and_target(a, b, test.getFilePath(), commands) 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) + target.copyFrom(source) return f"update-diff-test: copied {source} to {target}" diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py index b296f83bfbfa1..6f4db5b1b134e 100644 --- a/llvm/utils/lit/lit/TestRunner.py +++ b/llvm/utils/lit/lit/TestRunner.py @@ -1175,7 +1175,7 @@ def executeScriptInternal( ): for test_updater in litConfig.test_updaters: try: - update_output = test_updater(result, test) + update_output = test_updater(result, test, commands) except Exception as e: output = out output += err diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore b/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore index dd373bf9e0c66..aea8ee3be4982 100644 --- a/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore @@ -1,2 +1,10 @@ ; diff-tmp-dir.test clobbers this file empty.txt +; these test cases are clobbered when run, so they're recreated each time +single-split-file.test +single-split-file-populated.test +multiple-split-file.test +multiple-split-file-populated.test +single-split-file-no-expected.test +split-c-comments.test +split whitespace.test diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file-populated.in b/llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file-populated.in new file mode 100644 index 0000000000000..e218ed6a0c6ea --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file-populated.in @@ -0,0 +1,17 @@ +# RUN: split-file %s %t +# RUN: cp %S/1.in %t/out.txt +# RUN: diff %t/test3.expected %t/out.txt + +#--- test1.expected +unrelated +#--- test2.expected +#--- test3.expected +BAR + +BAZ + +#--- test4.expected +filler +#--- test5.expected + + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file.in b/llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file.in new file mode 100644 index 0000000000000..c47db99912c24 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file.in @@ -0,0 +1,13 @@ +# RUN: split-file %s %t +# RUN: cp %S/1.in %t/out.txt +# RUN: diff %t/test3.expected %t/out.txt + +#--- test1.expected +unrelated +#--- test2.expected +#--- test3.expected +#--- test4.expected +filler +#--- test5.expected + + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file.out b/llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file.out new file mode 100644 index 0000000000000..c1d2782d3c2d4 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/multiple-split-file.out @@ -0,0 +1,14 @@ +# RUN: split-file %s %t +# RUN: cp %S/1.in %t/out.txt +# RUN: diff %t/test3.expected %t/out.txt + +#--- test1.expected +unrelated +#--- test2.expected +#--- test3.expected +FOO +#--- test4.expected +filler +#--- test5.expected + + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-no-expected.in b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-no-expected.in new file mode 100644 index 0000000000000..510dc7afba16b --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-no-expected.in @@ -0,0 +1,6 @@ +# RUN: split-file %s %t +# RUN: cp %S/1.in %t/out.txt +# RUN: diff %t/test.txt %t/out.txt + +#--- test.txt + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-no-expected.out b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-no-expected.out new file mode 100644 index 0000000000000..f52e3004aee15 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-no-expected.out @@ -0,0 +1,6 @@ +# RUN: split-file %s %t +# RUN: cp %S/1.in %t/out.txt +# RUN: diff %t/test.txt %t/out.txt + +#--- test.txt +FOO diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-populated.in b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-populated.in new file mode 100644 index 0000000000000..63042cf9b86bc --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file-populated.in @@ -0,0 +1,7 @@ +# RUN: split-file %s %t +# RUN: cp %S/1.in %t/out.txt +# RUN: diff %t/test.expected %t/out.txt + +#--- test.expected +BAR + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file.in b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file.in new file mode 100644 index 0000000000000..422ccf2ef6813 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file.in @@ -0,0 +1,5 @@ +# RUN: split-file %s %t +# RUN: cp %S/1.in %t/out.txt +# RUN: diff %t/test.expected %t/out.txt + +#--- test.expected diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file.out b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file.out new file mode 100644 index 0000000000000..5552ad328ec5c --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/single-split-file.out @@ -0,0 +1,6 @@ +# RUN: split-file %s %t +# RUN: cp %S/1.in %t/out.txt +# RUN: diff %t/test.expected %t/out.txt + +#--- test.expected +FOO diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/split-both.test b/llvm/utils/lit/tests/Inputs/diff-test-update/split-both.test new file mode 100644 index 0000000000000..f564f446cc94b --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/split-both.test @@ -0,0 +1,11 @@ +# RUN: split-file %s %t +# RUN: diff %t/split-both.expected %t/split-both.out + +# ignore the fact that it's called ".expected" +# when comparing two files originating in split-file + +#--- split-both.expected +FOO +#--- split-both.out +BAR + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/split-c-comments.in b/llvm/utils/lit/tests/Inputs/diff-test-update/split-c-comments.in new file mode 100644 index 0000000000000..3cda60118f5ba --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/split-c-comments.in @@ -0,0 +1,6 @@ +// RUN: split-file %s %t +// RUN: cp %S/1.in %t/out.txt +// RUN: diff %t/test.txt %t/out.txt +// +//--- test.txt + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/split-c-comments.out b/llvm/utils/lit/tests/Inputs/diff-test-update/split-c-comments.out new file mode 100644 index 0000000000000..5020804f198b1 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/split-c-comments.out @@ -0,0 +1,6 @@ +// RUN: split-file %s %t +// RUN: cp %S/1.in %t/out.txt +// RUN: diff %t/test.txt %t/out.txt +// +//--- test.txt +FOO diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/split-whitespace.in b/llvm/utils/lit/tests/Inputs/diff-test-update/split-whitespace.in new file mode 100644 index 0000000000000..ad48d2ae4953c --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/split-whitespace.in @@ -0,0 +1,6 @@ +// RUN: split-file "%s" "%t" +// RUN: cp %S/1.in "%t/out.txt" +// RUN: diff "%t/test.txt" "%t/out.txt" +// +//--- test.txt + diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/split-whitespace.out b/llvm/utils/lit/tests/Inputs/diff-test-update/split-whitespace.out new file mode 100644 index 0000000000000..cb28124101ac6 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/split-whitespace.out @@ -0,0 +1,6 @@ +// RUN: split-file "%s" "%t" +// RUN: cp %S/1.in "%t/out.txt" +// RUN: diff "%t/test.txt" "%t/out.txt" +// +//--- test.txt +FOO diff --git a/llvm/utils/lit/tests/Inputs/diff-test-update/unrelated-split.test b/llvm/utils/lit/tests/Inputs/diff-test-update/unrelated-split.test new file mode 100644 index 0000000000000..b04eff36721de --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/diff-test-update/unrelated-split.test @@ -0,0 +1,11 @@ +# the fact that this test runs split-file is unrelated +# to the diffed files + +# RUN: mkdir %t +# RUN: split-file %s %t +# RUN: cp %S/1.in %t/unrelated-split.expected +# RUN: cp %S/2.in %t/unrelated-split.txt +# RUN: diff %t/unrelated-split.expected %t/unrelated-split.txt + +#--- distraction.txt + diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py b/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py index 0fda62c832f08..5b39d208a2ed6 100644 --- a/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py +++ b/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py @@ -1,2 +1,2 @@ -def should_not_run(foo, bar): +def should_not_run(foo, bar, baz): raise Exception("this test updater should only run on failure") diff --git a/llvm/utils/lit/tests/diff-test-update.py b/llvm/utils/lit/tests/diff-test-update.py index c37d0dccc727c..ad14034a85a17 100644 --- a/llvm/utils/lit/tests/diff-test-update.py +++ b/llvm/utils/lit/tests/diff-test-update.py @@ -1,10 +1,29 @@ +# RUN: cp %S/Inputs/diff-test-update/single-split-file.in %S/Inputs/diff-test-update/single-split-file.test +# RUN: cp %S/Inputs/diff-test-update/single-split-file-populated.in %S/Inputs/diff-test-update/single-split-file-populated.test +# RUN: cp %S/Inputs/diff-test-update/multiple-split-file.in %S/Inputs/diff-test-update/multiple-split-file.test +# RUN: cp %S/Inputs/diff-test-update/multiple-split-file-populated.in %S/Inputs/diff-test-update/multiple-split-file-populated.test +# RUN: cp %S/Inputs/diff-test-update/single-split-file-no-expected.in %S/Inputs/diff-test-update/single-split-file-no-expected.test +# RUN: cp %S/Inputs/diff-test-update/split-c-comments.in %S/Inputs/diff-test-update/split-c-comments.test +# RUN: cp %S/Inputs/diff-test-update/split-whitespace.in "%S/Inputs/diff-test-update/split whitespace.test" + # RUN: not %{lit} --update-tests -v %S/Inputs/diff-test-update | FileCheck %s +# RUN: diff %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file.test +# RUN: diff %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file-populated.test +# RUN: diff %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file.test +# RUN: diff %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file-populated.test +# RUN: diff %S/Inputs/diff-test-update/single-split-file-no-expected.out %S/Inputs/diff-test-update/single-split-file-no-expected.test +# RUN: diff %S/Inputs/diff-test-update/split-c-comments.out %S/Inputs/diff-test-update/split-c-comments.test +# RUN: diff %S/Inputs/diff-test-update/split-whitespace.out "%S/Inputs/diff-test-update/split whitespace.test" + + # CHECK: # update-diff-test: could not deduce source and target from {{.*}}1.in and {{.*}}2.in # CHECK: # update-diff-test: could not deduce source and target from {{.*}}1.txt and {{.*}}2.txt # CHECK: # update-diff-test: copied {{.*}}my-file.txt to {{.*}}my-file.expected # CHECK: # update-diff-test: copied {{.*}}1.txt to {{.*}}empty.txt # CHECK: # update-diff-test: copied {{.*}}diff-tmp.test.tmp.txt to {{.*}}diff-t-out.txt +# CHECK: # update-diff-test: could not deduce source and target from {{.*}}split-both.expected and {{.*}}split-both.out +# CHECK: # update-diff-test: copied {{.*}}unrelated-split.txt to {{.*}}unrelated-split.expected -# CHECK: Failed: 5 (100.00%) +# CHECK: Failed: 14 (100.00%) diff --git a/llvm/utils/lit/tests/pass-test-update.py b/llvm/utils/lit/tests/pass-test-update.py index 00a4025be660e..2e9f1be2bccab 100644 --- a/llvm/utils/lit/tests/pass-test-update.py +++ b/llvm/utils/lit/tests/pass-test-update.py @@ -12,7 +12,7 @@ # CHECK: Exception occurred in test updater: # CHECK: Traceback (most recent call last): # CHECK: File {{.*}}, line {{.*}}, in {{.*}} -# CHECK: update_output = test_updater(result, test) +# CHECK: update_output = test_updater(result, test, commands) # CHECK: File "{{.*}}{{/|\\}}should_not_run.py", line {{.*}}, in should_not_run # CHECK: raise Exception("this test updater should only run on failure") # CHECK: Exception: this test updater should only run on failure diff --git a/llvm/utils/update_any_test_checks.py b/llvm/utils/update_any_test_checks.py index 76fe336593929..ec277f140a34f 100755 --- a/llvm/utils/update_any_test_checks.py +++ b/llvm/utils/update_any_test_checks.py @@ -63,7 +63,7 @@ def expand_listfile_args(arg_list): return exp_arg_list -def utc_lit_plugin(result, test): +def utc_lit_plugin(result, test, commands): testname = test.getFilePath() if not testname: return None From 096ad6aa9142461e5532aaa6136d37e2ea3d3080 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Thu, 11 Sep 2025 15:49:30 -0700 Subject: [PATCH 08/10] [Utils] Compare true file locations instead of string paths (#158160) Previously we compared paths by string manipulation, however Windows paths can use both '\' and '/' as path separators, which made this fragile. This uses the pathlib.Path.samefile API instead. (cherry picked from commit b0181514b4d2a5f61ae5b405ee32643e6b8ff71b) --- llvm/utils/lit/lit/DiffUpdater.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/llvm/utils/lit/lit/DiffUpdater.py b/llvm/utils/lit/lit/DiffUpdater.py index 5bba2d70991df..fefcdcc99f3f2 100644 --- a/llvm/utils/lit/lit/DiffUpdater.py +++ b/llvm/utils/lit/lit/DiffUpdater.py @@ -1,6 +1,7 @@ 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. @@ -76,14 +77,12 @@ def get_target_dir(commands, test_path): @staticmethod def create(path, commands, test_path, target_dir): - filename = path.replace(target_dir, "") - if filename.startswith(os.sep): - filename = filename[len(os.sep) :] + 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 == filename: + if p and path.samefile(os.path.join(target_dir, p)): idx = i break else: From 5c01823981ac2a3a95a05f9f9d02266d1e6dcea8 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Fri, 12 Sep 2025 01:47:57 -0700 Subject: [PATCH 09/10] [Utils] fix diff_test_updater on Windows (#158235) (cherry picked from commit 1b05212acc1964837135930a129ee26e1a392278) --- llvm/utils/lit/lit/DiffUpdater.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/llvm/utils/lit/lit/DiffUpdater.py b/llvm/utils/lit/lit/DiffUpdater.py index fefcdcc99f3f2..a29c46fb8508f 100644 --- a/llvm/utils/lit/lit/DiffUpdater.py +++ b/llvm/utils/lit/lit/DiffUpdater.py @@ -62,17 +62,19 @@ def __str__(self): @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) + 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 - if split[1].strip() != test_path: + p = unquote(split[1].strip()) + if not test_path.samefile(p): continue - return split[2].strip() + return unquote(split[2].strip()) return None @staticmethod @@ -104,6 +106,12 @@ def _get_split_line_path(l): 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. @@ -145,7 +153,7 @@ def diff_test_updater(result, test, commands): [cmd, a, b] = args if cmd != "diff": return None - res = get_source_and_target(a, b, test.getFilePath(), commands) + 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 From 1de129d928540cd25d69451851fb482ab13eb254 Mon Sep 17 00:00:00 2001 From: dyung Date: Fri, 12 Sep 2025 15:52:50 +0100 Subject: [PATCH 10/10] Fix test on Windows by telling diff to ignore Windows-specific line endings. (#158297) Should fix bot: https://lab.llvm.org/buildbot/#/builders/46/builds/23206 (cherry picked from commit b0cb4e17e6ee362bbd8311adf2da7f3acb625fee) --- llvm/utils/lit/tests/diff-test-update.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/llvm/utils/lit/tests/diff-test-update.py b/llvm/utils/lit/tests/diff-test-update.py index ad14034a85a17..8b9f4610f7f95 100644 --- a/llvm/utils/lit/tests/diff-test-update.py +++ b/llvm/utils/lit/tests/diff-test-update.py @@ -8,13 +8,13 @@ # RUN: not %{lit} --update-tests -v %S/Inputs/diff-test-update | FileCheck %s -# RUN: diff %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file.test -# RUN: diff %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file-populated.test -# RUN: diff %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file.test -# RUN: diff %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file-populated.test -# RUN: diff %S/Inputs/diff-test-update/single-split-file-no-expected.out %S/Inputs/diff-test-update/single-split-file-no-expected.test -# RUN: diff %S/Inputs/diff-test-update/split-c-comments.out %S/Inputs/diff-test-update/split-c-comments.test -# RUN: diff %S/Inputs/diff-test-update/split-whitespace.out "%S/Inputs/diff-test-update/split whitespace.test" +# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file.test +# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file-populated.test +# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file.test +# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file-populated.test +# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/single-split-file-no-expected.out %S/Inputs/diff-test-update/single-split-file-no-expected.test +# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/split-c-comments.out %S/Inputs/diff-test-update/split-c-comments.test +# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/split-whitespace.out "%S/Inputs/diff-test-update/split whitespace.test" # CHECK: # update-diff-test: could not deduce source and target from {{.*}}1.in and {{.*}}2.in