-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Reland "[Utils] Add new --update-tests flag to llvm-lit" #153821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-testing-tools @llvm/pr-subscribers-clang Author: Henrik G. Olsson (hnrklssn) ChangesThis reverts commit e495231 to reland It also adds support for auto-fixing tests failing based on the Full diff: https://github.com/llvm/llvm-project/pull/153821.diff 20 Files Affected:
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))
|
✅ With the latest revision this PR passed the Python code formatter. |
c99dbb3
to
f19853e
Compare
This seems like an independent feature -- do it as a followup? |
Will do |
This reverts commit e495231 to reland the --update-tests feature, originally landed in llvm#108425.
f19853e
to
b2bdc5b
Compare
Please refrain from force push unless necessary: https://llvm.org/docs/GitHub.html#rebasing-pull-requests-and-force-pushes |
Deferred the diff test update changes to #154147 |
It looks like this also ends up regenerating XFAIL tests, which is not desirable. |
Thanks for letting me know! Will address in a follow up once #154147 is merged since that makes it easier to write test cases. |
Thanks for relanding this, it's super useful. It seems to rerun update_test_checks.py on tests that weren't failing, which can cause some spurious diffs that are just UTC regens. Is it possible to restrict this to only update the tests that failed? |
Are these XFAIL tests, or just normal tests that passed? |
Just normal tests that passed. Is this intentional? |
Definitely not. I'll take a look. |
Thanks! If it's of any help, here's one test where running |
This reverts commit llvm@e495231 to reland the --update-tests feature, originally landed in llvm#108425. (cherry picked from commit e1ff432)
This reverts commit llvm@e495231 to reland the --update-tests feature, originally landed in llvm#108425. (cherry picked from commit e1ff432)
This reverts commit llvm@e495231 to reland the --update-tests feature, originally landed in llvm#108425. (cherry picked from commit e1ff432)
This reverts commit e495231 to reland
the --update-tests feature, originally landed in #108425.
It also adds support for auto-fixing tests failing based on the
diff
command.