Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions llvm/utils/lit/lit/DiffUpdater.py
Original file line number Diff line number Diff line change
@@ -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}"
3 changes: 2 additions & 1 deletion llvm/utils/lit/lit/LitConfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -93,7 +94,7 @@ def __init__(
self.per_test_coverage = per_test_coverage
self.gtest_sharding = bool(gtest_sharding)
self.update_tests = update_tests
self.test_updaters = []
self.test_updaters = [diff_test_updater]

@property
def maxIndividualTestTime(self):
Expand Down
1 change: 1 addition & 0 deletions llvm/utils/lit/tests/Inputs/diff-test-update/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
*.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

%t's are unique, so where does this take effect?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp-dir.test creates an empty file at %S/empty.txt to have a file outside %t (to test the heuristic that if one file is in %t and the other isn't, the %t file is the one to copy from) that can be overwritten by the test without triggering a git diff. But now that you mention it I realise that needs to be erased if it has been written to by a previous test run... BRB creating a follow-up

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I could also make it more explicit and mark only empty.txt as gitignored

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow-up: #157576

1 change: 1 addition & 0 deletions llvm/utils/lit/tests/Inputs/diff-test-update/1.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
FOO
1 change: 1 addition & 0 deletions llvm/utils/lit/tests/Inputs/diff-test-update/2.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
BAR
3 changes: 3 additions & 0 deletions llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail.test
Original file line number Diff line number Diff line change
@@ -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

7 changes: 7 additions & 0 deletions llvm/utils/lit/tests/Inputs/diff-test-update/diff-bail2.test
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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

3 changes: 3 additions & 0 deletions llvm/utils/lit/tests/Inputs/diff-test-update/diff-tmp.test
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions llvm/utils/lit/tests/Inputs/diff-test-update/lit.cfg
Original file line number Diff line number Diff line change
@@ -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

10 changes: 10 additions & 0 deletions llvm/utils/lit/tests/diff-test-update.py
Original file line number Diff line number Diff line change
@@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CHECK lines need to be updated to account for Windows path separators.

https://lab.llvm.org/buildbot/#/builders/46/builds/22989/steps/7/logs/FAIL__lit___diff-test-update_py

# update-diff-test: could not deduce source and target from Z:\b\llvm-clang-x86_64-sie-win\build\utils\lit\tests\Inputs\diff-test-update/1.in and Z:\b\llvm-clang-x86_64-sie-win\build\utils\lit\tests\Inputs\diff-test-update/2.in

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, should be addressed in #157576

# 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%)