llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Henrik G. Olsson (hnrklssn) <details> <summary>Changes</summary> 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. It also adds support for auto-fixing tests failing based on the `diff` command. --- Full diff: https://github.com/llvm/llvm-project/pull/153821.diff 20 Files Affected: - (modified) clang/test/lit.cfg.py (+10) - (modified) llvm/docs/CommandGuide/lit.rst (+5) - (modified) llvm/test/lit.cfg.py (+10) - (added) llvm/utils/lit/lit/DiffUpdater.py (+36) - (modified) llvm/utils/lit/lit/LitConfig.py (+4) - (modified) llvm/utils/lit/lit/TestRunner.py (+12) - (modified) llvm/utils/lit/lit/cl_arguments.py (+6) - (modified) llvm/utils/lit/lit/llvm/config.py (+5) - (modified) llvm/utils/lit/lit/main.py (+1) - (added) llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore (+1) - (added) llvm/utils/lit/tests/Inputs/diff-test-update/1.in (+1) - (added) llvm/utils/lit/tests/Inputs/diff-test-update/2.in (+1) - (added) llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail.test (+3) - (added) llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail2.test (+7) - (added) llvm/utils/lit/tests/Inputs/diff-test-update/diff-expected.test (+5) - (added) llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp-dir.test (+6) - (added) llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp.test (+3) - (added) llvm/utils/lit/tests/Inputs/diff-test-update/lit.cfg (+8) - (added) llvm/utils/lit/tests/diff-test-update.py (+10) - (modified) llvm/utils/update_any_test_checks.py (+51-3) ``````````diff 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..9027a93c5f400 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.update_tests`` 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/DiffUpdater.py b/llvm/utils/lit/lit/DiffUpdater.py new file mode 100644 index 0000000000000..e2bc5a63abcbd --- /dev/null +++ b/llvm/utils/lit/lit/DiffUpdater.py @@ -0,0 +1,36 @@ +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 cb4aef6f72a87..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): @@ -39,6 +40,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 +93,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): 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/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%) 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)) `````````` </details> https://github.com/llvm/llvm-project/pull/153821 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
