[llvm] [llvm-lit] Add precommit test to verify current behavior of glob expansion in lit's internal shell (PR #106325)

via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 27 19:20:35 PDT 2024


https://github.com/Harini0924 created https://github.com/llvm/llvm-project/pull/106325

This patch introduces a precommit test to verify the current behavior of glob expansion in lit's internal shell. The motivation for this test stems from an issue encountered during the BOLT test suite when running with the lit internal shell using the command:

`LIT_USE_INTERNAL_SHELL=1 ninja check-bolt`

During execution, the following error was observed:
```
File "/usr/local/google/home/harinidonthula/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 416, in executeBuiltinEcho
    stdout.write(encode(maybeUnescape(args[-1])))
TypeError: string argument expected, got 'GlobItem'
```
The `executeBuiltinEcho` function in the lit testing framework expects a string to be passed to `stdout.write`, but it received a `GlobItem` object instead. This precommit test is designed to check the current behavior where the glob pattern isn't correctly expanded, leading to this `TypeError`.

While this patch doesn't fix the issue, it helps in understanding and verifying the current behavior. The feedback I received from this [PR](https://github.com/llvm/llvm-project/pull/105925) suggests using `cmd.args = expand_glob_expressions(cmd.args, shenv.cwd)` to match the behavior of `executeBuiltinMkdir` and `executeBuiltinRm`, but it is recognized that the internal shell should ideally expand globs before calling any built-in command.

**Request for Feedback:**
I'm looking for feedback on how to improve this precommit test, specifically regarding the handling and expansion of glob patterns for commands like mkdir and rm within the internal shell. Currently, the args are expanded at the beginning of these functions, which should ensure proper glob expansion. However, I'd appreciate guidance on whether I should write additional tests to verify that mkdir and rm are handling glob expansions correctly.

If such tests are recommended, I would also appreciate advice on the best approach to implement them, considering the existing framework and the way glob expansion is expected to function in the internal shell. Should these tests confirm that the current implementation passes, or are there specific edge cases I should be aware of?

**Next Steps:**

In my follow-up PR, I plan to address the UNRESOLVED error by expanding the entire command, ensuring correct and consistent behavior across all commands. The current test checks for an unresolved issue with the glob expansion, specifically looking for a `TypeError` due to an unexpanded `GlobItem`. This will be updated to reflect the correct behavior once the issue is resolved.

This change is relevant for [[RFC] Enabling the Lit Internal Shell by Default](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179/3)  


>From 36c5ece458b0038e2127e8be4d517f2ec5dc9181 Mon Sep 17 00:00:00 2001
From: Harini <harinidonthula at google.com>
Date: Wed, 28 Aug 2024 01:48:41 +0000
Subject: [PATCH] [llvm-lit] Add shtest-glob.py test to verify glob pattern
 handling

This commit introduces a new test file, shtest-glob.py, to the lit testing framework. The test is designed to verify the handling of glob patterns within the internal shell, specifically checking the behavior of commands like echo when glob patterns are present. The test includes example files, example_file1.txt and example_file2.txt, which are used to validate whether the glob expansion is correctly performed or remains unexpanded as expected in certain cases. This addition helps ensure that glob pattern processing behaves consistently across different shell commands in lit.
---
 .../utils/lit/tests/Inputs/shtest-glob/example_file1.txt | 2 ++
 .../utils/lit/tests/Inputs/shtest-glob/example_file2.txt | 2 ++
 llvm/utils/lit/tests/Inputs/shtest-glob/glob-echo.txt    | 2 ++
 llvm/utils/lit/tests/shtest-glob.py                      | 9 +++++++++
 4 files changed, 15 insertions(+)
 create mode 100644 llvm/utils/lit/tests/Inputs/shtest-glob/example_file1.txt
 create mode 100644 llvm/utils/lit/tests/Inputs/shtest-glob/example_file2.txt
 create mode 100644 llvm/utils/lit/tests/Inputs/shtest-glob/glob-echo.txt
 create mode 100644 llvm/utils/lit/tests/shtest-glob.py

diff --git a/llvm/utils/lit/tests/Inputs/shtest-glob/example_file1.txt b/llvm/utils/lit/tests/Inputs/shtest-glob/example_file1.txt
new file mode 100644
index 00000000000000..1bf50291a824a6
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-glob/example_file1.txt
@@ -0,0 +1,2 @@
+## This is the first example file used for testing glob pattern matching.
+RUN: This is the first example file.
diff --git a/llvm/utils/lit/tests/Inputs/shtest-glob/example_file2.txt b/llvm/utils/lit/tests/Inputs/shtest-glob/example_file2.txt
new file mode 100644
index 00000000000000..7263ea1dccf4a8
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-glob/example_file2.txt
@@ -0,0 +1,2 @@
+## This is the second example file used for testing glob pattern matching.
+RUN: This is the second example file.
diff --git a/llvm/utils/lit/tests/Inputs/shtest-glob/glob-echo.txt b/llvm/utils/lit/tests/Inputs/shtest-glob/glob-echo.txt
new file mode 100644
index 00000000000000..31fc03f22bd5f9
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-glob/glob-echo.txt
@@ -0,0 +1,2 @@
+## Tests glob pattern expansion by listing matching files.
+# RUN: echo %{inputs}/shtest-glob/example_file*.txt
diff --git a/llvm/utils/lit/tests/shtest-glob.py b/llvm/utils/lit/tests/shtest-glob.py
new file mode 100644
index 00000000000000..3d59e681cca4f9
--- /dev/null
+++ b/llvm/utils/lit/tests/shtest-glob.py
@@ -0,0 +1,9 @@
+## Tests glob pattern handling in echo command.
+
+# RUN: not %{lit} -a -v %{inputs}/shtest-glob \ 
+# RUN: | FileCheck -dump-input=fail -match-full-lines %s
+#
+# END.
+
+# CHECK: UNRESOLVED: shtest-glob :: glob-echo.txt ({{[^)]*}})
+# CHECK: TypeError: string argument expected, got 'GlobItem'



More information about the llvm-commits mailing list