[lit] Allow passing extra commands to executeShTest

[ldionne]

Summary

This allows creating custom test formats on top of executeShTest that inject commands at the beginning of the file being parsed, without requiring these commands to physically appear in the test file itself.

For example, one could define a test format that prints out additional debug information at the beginning of each test. More realistically, this has been used to define custom test formats like one that supports compilation failure tests (e.g. with the extension compile.fail.cpp) by injecting a command that calls the compiler on the file itself and expects it to fail.

Without this change, the only alternative is to create a temporary file with the same content as the original test, then prepend the desired // RUN: lines to that file, and call executeShTest on that file instead. This is both slow and cumbersome to do.

[ldionne]

I couldn't find existing unit tests for executeShTest that I could have added to, but please LMK if there are some.

[yln]

I don't understand yet what this enables. Please extend your commit message to elaborate a bit more for people (like me) who aren't familiar with libcxx testing.
Does this enable deriving new tests from existing source files or running existing tests (.{pass,fail,sh}.cpp) in a different way without needing to modify those dozens/hundreds of files?

The change itself looks innocent enough and I don't want to block you on adding a test if there are currently none at all. LGTM.

[yln]

llvm/utils/lit/lit/TestRunner.py

1497

I think Python "truthiness" allows for not script here.

[dexonsmith]

In D76290#1930134, @yln wrote:

I don't understand yet what this enables. Please extend your commit message to elaborate a bit more for people (like me) who aren't familiar with libcxx testing.
Does this enable deriving new tests from existing source files or running existing tests (.{pass,fail,sh}.cpp) in a different way without needing to modify those dozens/hundreds of files?

The change itself looks innocent enough and I don't want to block you on adding a test if there are currently none at all. LGTM.

I'm a little uncomfortable with declaring bankruptcy on the tests. Doesn't the lit test suite itself use executeShTest? Can we add a test there?

This would also be great to document if there's a place. If there isn't, maybe we can change (maybe just one of) the libcxx tests to use this in the same commit, so that it's obvious in a blame what this is for.

[ldionne]

Add tests, reword commit, use Python truthiness to full extent

[ldionne]

In D76290#1930134, @yln wrote:

I don't understand yet what this enables.

It enables injecting commands at the beginning of an arbitrary file, even if there are no commands in that file. That makes the ShTest format a lot more powerful, cause new formats can be built on top of it. For example, this is the new test format I'm working on for libc++, which is both more powerful and *a lot* simpler than the current test format: https://github.com/ldionne/llvm-project/blob/a60ed4470/libcxx/utils/libcxx/test/newformat.py#L79-L138.

Without this change, I have to create a temporary file for each test, copy it, prepend // RUN: XXX commands to it, and then re-create an artificial Lit test that uses that file. It's not great and it's a lot slower.

Please extend your commit message to elaborate a bit more for people (like me) who aren't familiar with libcxx testing.

I've tried to better describe what the change enables.

In D76290#1930209, @dexonsmith wrote:

I'm a little uncomfortable with declaring bankruptcy on the tests. Doesn't the lit test suite itself use executeShTest? Can we add a test there?

I agree, and you're very good at making me write tests :-). I'm now more familiar with how lit tests work and I've managed to add some.

@dexonsmith @yln
Please let me know if the new revision seems OK to you!

[delcypher]

llvm/utils/lit/lit/TestRunner.py

1492

I don't think extra_commands is a command good name. It's not obvious that these are injected as extra commands at the beginning. Here are a few name suggestions

• preamble_commands
• prologue_commands

[ldionne]

Renamed to preamble_commands

[ldionne]

I renamed to preamble_commands, @delcypher LMK if that looks better now!

[delcypher]

llvm/utils/lit/tests/shtest-inject.py

2

These tests require an external shell. I don't know if that option is available on all the platforms where this test runs (e.g. Windows). Do you need to put a REQUIRES: <something> on this test?

[ldionne]

llvm/utils/lit/tests/shtest-inject.py

2

These tests follow the exact format of the other tests in this directory, so this must not be an issue. Can I commit this?

[delcypher]

llvm/utils/lit/tests/shtest-inject.py

2

Ok.

llvm/utils/lit/lit/TestRunner.py

if out:
         output += """Command Output (stdout):\n--\n%s\n--\n""" % (out,)
     if err:
         output += """Command Output (stderr):\n--\n%s\n--\n""" % (err,)
 
     return lit.Test.Result(status, output)
 
 
 def executeShTest(test, litConfig, useExternalSh,
-                  extra_substitutions=[]):
+                  extra_substitutions=[],
+                  preamble_commands=[]):
     if test.config.unsupported:
         return lit.Test.Result(Test.UNSUPPORTED, 'Test is unsupported')
 
-    script = parseIntegratedTestScript(test)
-    if isinstance(script, lit.Test.Result):
-        return script
+    script = list(preamble_commands)
+    parsed = parseIntegratedTestScript(test, require_script=not script)
+    if isinstance(parsed, lit.Test.Result):
+        return parsed
+    script += parsed
+
     if litConfig.noExecute:
         return lit.Test.Result(Test.PASS)
 
     tmpDir, tmpBase = getTempPaths(test)
     substitutions = list(extra_substitutions)
     substitutions += getDefaultSubstitutions(test, tmpDir, tmpBase,
                                              normalize_slashes=useExternalSh)
     script = applySubstitutions(script, substitutions)

llvm/utils/lit/tests/Inputs/shtest-inject/lit.cfg

+import lit
+
+class CustomFormat(lit.formats.TestFormat):
+    def execute(self, test, litConfig):
+        commands = [
+            'echo "THIS WAS"',
+            'echo "INJECTED"'
+        ]
+        return lit.TestRunner.executeShTest(test, litConfig,
+                                            useExternalSh=False,
+                                            preamble_commands=commands)
+
+config.name = 'shtest-inject'
+config.suffixes = ['.txt']
+config.test_format = CustomFormat()
+config.test_source_root = None
+config.test_exec_root = None

llvm/utils/lit/tests/Inputs/shtest-inject/test-empty.txt

+
+# This test voluntarily has no RUN lines or anything else. The RUN lines are
+# injected by the test format.

llvm/utils/lit/tests/Inputs/shtest-inject/test-many.txt

+
+# This test has several RUN lines, but more run lines are prepended to it by
+# the test format in use.
+
+# RUN: echo "IN THE FILE"
+# RUN: echo "IF IT WORKS"
+# RUN: echo "AS EXPECTED"

llvm/utils/lit/tests/Inputs/shtest-inject/test-one.txt

+
+# This test has one RUN line, but more run lines are prepended to it by the
+# test format in use.
+
+# RUN: echo "IN THE FILE"

llvm/utils/lit/tests/shtest-inject.py

+# Check that we can inject commands at the beginning of a ShTest using a custom
+# test format.
+
+# RUN: %{lit} -j 1 %{inputs}/shtest-inject/test-empty.txt --show-all | FileCheck --check-prefix=CHECK-TEST1 %s
+#
+# CHECK-TEST1: Script:
+# CHECK-TEST1: --
+# CHECK-TEST1: echo "THIS WAS"
+# CHECK-TEST1: echo "INJECTED"
+# CHECK-TEST1: --
+#
+# CHECK-TEST1: THIS WAS
+# CHECK-TEST1: INJECTED
+#
+# CHECK-TEST1: Expected Passes    : 1
+
+# RUN: %{lit} -j 1 %{inputs}/shtest-inject/test-one.txt --show-all | FileCheck --check-prefix=CHECK-TEST2 %s
+#
+# CHECK-TEST2: Script:
+# CHECK-TEST2: --
+# CHECK-TEST2: echo "THIS WAS"
+# CHECK-TEST2: echo "INJECTED"
+# CHECK-TEST2: echo "IN THE FILE"
+# CHECK-TEST2: --
+#
+# CHECK-TEST2: THIS WAS
+# CHECK-TEST2: INJECTED
+# CHECK-TEST2: IN THE FILE
+#
+# CHECK-TEST2: Expected Passes    : 1
+
+# RUN: %{lit} -j 1 %{inputs}/shtest-inject/test-many.txt --show-all | FileCheck --check-prefix=CHECK-TEST3 %s
+#
+# CHECK-TEST3: Script:
+# CHECK-TEST3: --
+# CHECK-TEST3: echo "THIS WAS"
+# CHECK-TEST3: echo "INJECTED"
+# CHECK-TEST3: echo "IN THE FILE"
+# CHECK-TEST3: echo "IF IT WORKS"
+# CHECK-TEST3: echo "AS EXPECTED"
+# CHECK-TEST3: --
+#
+# CHECK-TEST3: THIS WAS
+# CHECK-TEST3: INJECTED
+# CHECK-TEST3: IN THE FILE
+# CHECK-TEST3: IF IT WORKS
+# CHECK-TEST3: AS EXPECTED
+#
+# CHECK-TEST3: Expected Passes    : 1