[llvm-lit] Precommit Tests for implementing the unset command in lit internal shell
#104618
Conversation
This suite of tests is designed to verify the expected failures when attempting to use the unset command within the lit internal shell. These tests ensure that the unset command fails with the correct error messages and exit codes, demonstrating that the command is not recognized or supported in this environment. These failures are expected and validate the current behavior of the lit internal shell when handling the `unset` command.
|
@llvm/pr-subscribers-testing-tools Author: None (Harini0924) ChangesThis patch introduces a suite of tests designed to verify the current behavior of the In my following PR, I will implement support for the This change is relevant for [RFC] Enabling the Lit Internal Shell by Default Full diff: https://github.com/llvm/llvm-project/pull/104618.diff 6 Files Affected:
diff --git a/llvm/utils/lit/tests/Inputs/shtest-unset/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-unset/lit.cfg
new file mode 100644
index 00000000000000..ec8c43263be721
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-unset/lit.cfg
@@ -0,0 +1,10 @@
+import lit.formats
+
+config.name = "shtest-unset"
+config.suffixes = [".txt"]
+config.test_format = lit.formats.ShTest()
+config.test_source_root = None
+config.test_exec_root = None
+config.environment["FOO"] = "1"
+config.environment["BAR"] = "2"
+config.substitutions.append(("%{python}", '"%s"' % (sys.executable)))
diff --git a/llvm/utils/lit/tests/Inputs/shtest-unset/unset-multiple-variables.txt b/llvm/utils/lit/tests/Inputs/shtest-unset/unset-multiple-variables.txt
new file mode 100644
index 00000000000000..f7dc7e80bd93b4
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-unset/unset-multiple-variables.txt
@@ -0,0 +1,3 @@
+## Tests the 'unset' command with multiple variables.
+
+# RUN: unset FOO BAR
diff --git a/llvm/utils/lit/tests/Inputs/shtest-unset/unset-no-args.txt b/llvm/utils/lit/tests/Inputs/shtest-unset/unset-no-args.txt
new file mode 100644
index 00000000000000..bc9f27710c5bfb
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-unset/unset-no-args.txt
@@ -0,0 +1,3 @@
+## Tests the 'unset' command when no arguments are provided.
+
+# RUN: unset
diff --git a/llvm/utils/lit/tests/Inputs/shtest-unset/unset-nonexistent-variable.txt b/llvm/utils/lit/tests/Inputs/shtest-unset/unset-nonexistent-variable.txt
new file mode 100644
index 00000000000000..5fbe3fb757291b
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-unset/unset-nonexistent-variable.txt
@@ -0,0 +1,3 @@
+## Test the behavior of the 'unset' command when trying to unset a variable that does not exist.
+
+# RUN: unset NONEXISTENT
diff --git a/llvm/utils/lit/tests/Inputs/shtest-unset/unset-variable.txt b/llvm/utils/lit/tests/Inputs/shtest-unset/unset-variable.txt
new file mode 100644
index 00000000000000..11adcd234e7177
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-unset/unset-variable.txt
@@ -0,0 +1,3 @@
+## Tests the 'unset' command with a single variable
+
+# RUN: unset FOO
diff --git a/llvm/utils/lit/tests/shtest-unset.py b/llvm/utils/lit/tests/shtest-unset.py
new file mode 100644
index 00000000000000..fdcc2ed5d86fce
--- /dev/null
+++ b/llvm/utils/lit/tests/shtest-unset.py
@@ -0,0 +1,42 @@
+## Check that the 'unset' command fails as expected for various tests.
+
+# RUN: not %{lit} -a -v %{inputs}/shtest-unset \
+# RUN: | FileCheck -match-full-lines %s
+#
+# END.
+
+## Check that the 'unset' command's expected failures.
+
+# CHECK: -- Testing: 4 tests{{.*}}
+
+# CHECK: FAIL: shtest-unset :: unset-multiple-variables.txt{{.*}}
+# CHECK: unset FOO BAR
+# CHECK-NEXT: # executed command: unset FOO BAR
+# CHECK-NEXT: # .---command stderr------------
+# CHECK-NEXT: # | 'unset': command not found
+# CHECK: # error: command failed with exit status: 127
+
+# CHECK: FAIL: shtest-unset :: unset-no-args.txt{{.*}}
+# CHECK: unset
+# CHECK-NEXT: # executed command: unset
+# CHECK-NEXT: # .---command stderr------------
+# CHECK-NEXT: # | 'unset': command not found
+# CHECK: # error: command failed with exit status: 127
+
+# CHECK: FAIL: shtest-unset :: unset-nonexistent-variable.txt{{.*}}
+# CHECK: unset NONEXISTENT
+# CHECK-NEXT: # executed command: unset NONEXISTENT
+# CHECK-NEXT: # .---command stderr------------
+# CHECK-NEXT: # | 'unset': command not found
+# CHECK: # error: command failed with exit status: 127
+
+# CHECK: FAIL: shtest-unset :: unset-variable.txt{{.*}}
+# CHECK: unset FOO
+# CHECK-NEXT: # executed command: unset FOO
+# CHECK-NEXT: # .---command stderr------------
+# CHECK-NEXT: # | 'unset': command not found
+# CHECK: # error: command failed with exit status: 127
+
+# CHECK: Total Discovered Tests: 4
+# CHECK: Failed: 4 {{\([0-9]*\.[0-9]*%\)}}
+# CHECK-NOT: {{.}}
|
|
CC: @ilovepi @petrhosek |
|
Related to my most recent comment in the RFC, how many tests make use of |
…ssaging This patch introduces the executeBuiltinUnset function in TestRunner.py, which I created to handle the 'unset' command. The function raises an InternalShellError with a clear message advising users to use env -u VARIABLE as an alternative since the unset command is not supported by the lit internal shell. Additionally, the test in shtest-unset.py has been updated to verify this error message.
|
@jh7370 After reviewing the number of tests that make use of the However, this PR still checks the current state and will display an error message that guides users to use What are your thoughts on this patch? |
There was a problem hiding this comment.
Honestly, I'm not sure this is necessary. env -u is a standard Linux shell command, so in theory at least, it should be as well known as unset for those used to that style. I think the fact that only two tests use it, and both of those tests are linked tightly, shows that unset isn't something people tend to use, so the cost of extra complexity in lit (by virtue of having more code) doesn't really seem justified.
|
That makes sense. Given that |
This patch introduces a suite of tests designed to verify the current behavior of the
unsetcommand within the lit internal shell. As it stands, theunsetcommand is not recognized or supported in this environment, resulting in expected failures with specific error messages and exit codes. These tests are intentionally failing to reflect the current state. Additionally, I have added theexecuteBuiltinUnsetfunction, which raises anInternalShellErrorwith a clear message advising users to useenv -u VARIABLEas an alternative since theunsetcommand is not supported by the lit internal shell.This change is relevant for [RFC] Enabling the Lit Internal Shell by Default