Skip to content
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

Make update_any_test_check.py script accepting @listfile CL argument. #86800

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

vpykhtin
Copy link
Contributor

Usage: update_any_test_check.py @my_list_of_tests
where my_list_of_tests is a file containing list of tests to update.

Sorry for possible inefficiencies I'm a chatgpt powered python newbie.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 27, 2024

@llvm/pr-subscribers-testing-tools

Author: Valery Pykhtin (vpykhtin)

Changes

Usage: update_any_test_check.py @<!-- -->my_list_of_tests
where my_list_of_tests is a file containing list of tests to update.

Sorry for possible inefficiencies I'm a chatgpt powered python newbie.


Full diff: https://github.com/llvm/llvm-project/pull/86800.diff

1 Files Affected:

  • (modified) llvm/utils/update_any_test_checks.py (+21-2)
diff --git a/llvm/utils/update_any_test_checks.py b/llvm/utils/update_any_test_checks.py
index 2ad852d710c07a..da3290415245b1 100755
--- a/llvm/utils/update_any_test_checks.py
+++ b/llvm/utils/update_any_test_checks.py
@@ -4,7 +4,8 @@
 
 Given a list of test files, this script will invoke the correct
 update_test_checks-style script, skipping any tests which have not previously
-had assertions autogenerated.
+had assertions autogenerated. If test name starts with '@' it's treated as
+a name of file containing test list.
 """
 
 from __future__ import print_function
@@ -39,6 +40,22 @@ def run_utc_tool(utc_name, utc_tool, testname):
     )
     return (result.returncode, result.stdout, result.stderr)
 
+def read_arguments_from_file(filename):
+    try:
+        with open(filename, 'r') as file:
+            return [line.rstrip() for line in file.readlines()]
+    except FileNotFoundError:
+        print(f"Error: File '{filename}' not found.")
+        sys.exit(1)
+
+def expand_listfile_args(arg_list):
+    exp_arg_list = []
+    for arg in arg_list:
+        if arg.startswith('@'):
+            exp_arg_list += read_arguments_from_file(arg[1:])
+        else:
+            exp_arg_list.append(testname)
+    return exp_arg_list
 
 def main():
     from argparse import RawTextHelpFormatter
@@ -72,10 +89,12 @@ def main():
     utc_tools = {}
     have_error = False
 
+    tests = expand_listfile_args(config.tests)
+
     with ThreadPoolExecutor(max_workers=config.jobs) as executor:
         jobs = []
 
-        for testname in config.tests:
+        for testname in tests:
             with open(testname, "r") as f:
                 header = f.readline().strip()
                 m = RE_ASSERTIONS.search(header)

Copy link

github-actions bot commented Mar 27, 2024

✅ With the latest revision this PR passed the Python code formatter.

Copy link
Collaborator

@nhaehnle nhaehnle left a comment

Choose a reason for hiding this comment

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

Please do take a look at the formatting complaint.

Is their precedent somewhere for using the '@' as a prefix in this way? Seems a bit unorthodox to me. Perhaps because a more "CLI idiomatic" way of achieving the same thing is to just run something like cat yourfile | xargs update_any_test_checks.py or update_any_test_checks.py $(cat yourfile).

@tru
Copy link
Collaborator

tru commented Apr 17, 2024

Is their precedent somewhere for using the '@' as a prefix in this way? Seems a bit unorthodox to me.

This is handled by lld and clang as well. It's called response files there and is usually a way to work around issues with command line length.

@vpykhtin
Copy link
Contributor Author

Thanks for the comments. We don't have CL length problem but it's sometimes convenient to have a separate file with a list of tests to proceed.

@nhaehnle
Copy link
Collaborator

Okay. Please address the formatting issues, the rest LGTM.

@vpykhtin
Copy link
Contributor Author

Passed formatting check.

@vpykhtin vpykhtin merged commit 33786b6 into llvm:main Apr 18, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants