[llvm-lit][test] Precommit tests for curly braces in lit internal shell

[connieyzhu]

This patch creates tests to check lit's failure to execute curly braces {}. This is a prequisite patch to #102830.

[llvmbot]

@llvm/pr-subscribers-testing-tools

Author: Connie (connieyzhu)

Changes

This patch creates tests to check lit's failure to execute curly braces {}. This is a prequisite patch to #102830.

---

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

2 Files Affected:

• (added) llvm/utils/lit/tests/Inputs/shtest-shell/curly-brace.txt (+12)
• (modified) llvm/utils/lit/tests/shtest-shell.py (+8-2)

diff --git a/llvm/utils/lit/tests/Inputs/shtest-shell/curly-brace.txt b/llvm/utils/lit/tests/Inputs/shtest-shell/curly-brace.txt
new file mode 100644
index 00000000000000..b554b9772db89d
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-shell/curly-brace.txt
@@ -0,0 +1,12 @@
+## Test curly braces used in grouping commands.
+
+## Test one command inside curly brace.
+# RUN: { echo bar; } | FileCheck --check-prefix=ONE-CMD
+
+# ONE-CMD: bar
+
+## Test two commands inside curly brace.
+# RUN: { echo foo; echo bar; } | FileCheck --check-prefix=TWO-CMDS
+
+# TWO-CMDS: foo
+# TWO-CMDS: bar
\ No newline at end of file
diff --git a/llvm/utils/lit/tests/shtest-shell.py b/llvm/utils/lit/tests/shtest-shell.py
index 86851194880620..a050a57eea64bb 100644
--- a/llvm/utils/lit/tests/shtest-shell.py
+++ b/llvm/utils/lit/tests/shtest-shell.py
@@ -4,7 +4,7 @@
 # FIXME: Temporarily dump test output so we can debug failing tests on
 # buildbots.
 # RUN: cat %t.out
-# RUN: FileCheck --input-file %t.out %s
+# RUN: FileCheck --input-file %t.out %s --dump-input-context=1000
 #
 # Test again in non-UTF shell to catch potential errors with python 2 seen
 # on stdout-encoding.txt
@@ -44,6 +44,12 @@
 
 # CHECK: PASS: shtest-shell :: continuations.txt
 
+# CHECK: FAIL: shtest-shell :: curly-brace.txt
+# CHECK: # executed command: '{' echo bar
+# CHECK-NEXT: # .---command stderr------------
+# CHECK-NEXT: # | '{': command not found
+# CHECK: error: command failed with exit status: 127
+
 # CHECK: PASS: shtest-shell :: dev-null.txt
 
 #      CHECK: FAIL: shtest-shell :: diff-b.txt
@@ -651,4 +657,4 @@
 
 # CHECK: PASS: shtest-shell :: valid-shell.txt
 # CHECK: Unresolved Tests (1)
-# CHECK: Failed Tests (38)
+# CHECK: Failed Tests (39)

[connieyzhu]

CC: @ilovepi @petrhosek @jh7370

Hi there, I'm writing some tests for curly braces in lit's internal shell before I implement the functionality in #102830. I wasn't sure if I should write more in-depth tests in this patch, since the same syntax error will be given for every test at the moment, or if I should just include more tests in my later patch once I have implemented the proper functionality.

[ilovepi]

There's some value in showing that each test used to fail, but now passes. I'm not sure it needs to be particularly exhaustive, but whatever basic tests you're considering are probably reasonable to include in this patch. Typically its nice if none of the RUN lines change, but you can easily spot a difference in the test behavior. I know I appreciate that in the middle or backend tests I review.

[ilovepi]

I'm not sure this is checking things well enough. The nested {...} probably needs to result in something that's distinct from the regular case. As written, you cant distinguish the nested version from the un-nested version. Maybe you can use a | or file redirection + cat?

[ilovepi]

leftover from debugging?

[connieyzhu]

Yes, forgot to delete that. I will remove it in my next change.

[ilovepi]

I don't see a mkdir %t, so I'm not sure this works as intended. If you don't need other files, you can just use %t.

[ilovepi]

Suggested change

	# NESTED: bar
	# NESTED: baz
	# NESTED: foo
	# NESTED: bar
	# NESTED-NEXT: baz
	# NESTED-NEXT: foo

[jh7370]

I'm not opposed to this PR being developed, but I don't want it and the follow-on PR landed without the RFC discussion I've requested in the source issue: as noted there, I'm not convinced this functionality is needed in lit (not going to elaborate further here, to avoid splitting the discussion).