[llvm-lit][test] Precommit tests for lit's built-in cat command #101530
Conversation
This patch creates preliminary checks for lit's built-in cat command. These tests verify the cat and cat -v functionality, and display failures with using cat -e which will be implemented in a later patch.
|
@llvm/pr-subscribers-testing-tools Author: Connie (connieyzhu) ChangesThis patch creates preliminary checks for lit's built-in cat command. These tests verify the cat and cat -v functionality, and display failures with using cat -e which will be implemented in a later patch. Full diff: https://github.com/llvm/llvm-project/pull/101530.diff 8 Files Affected:
diff --git a/llvm/utils/lit/tests/Inputs/shtest-cat/cat-e-allchars.txt b/llvm/utils/lit/tests/Inputs/shtest-cat/cat-e-allchars.txt
new file mode 100644
index 0000000000000..6955b796f695a
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-cat/cat-e-allchars.txt
@@ -0,0 +1,19 @@
+# RUN: cat -e %S/txt-files/allchars | FileCheck %s
+
+# CHECK: ^@^A^B^C^D^E^F^G^H $
+# CHECK: ^K^L^M^N^O^P^Q^R^S^T^U^V^W^X^Y^Z
+# CHECK-SAME: ^[^\^]^^^_ !"#$%&'()*+,-./
+# CHECK-SAME: 0123456789
+# CHECK-SAME: :;<=>?@
+# CHECK-SAME: ABCDEFGHIJKLMNOPQRSTUVWXYZ
+# CHECK-SAME: [\]^_`
+# CHECK-SAME: abcdefghijklmnopqrstuvwxyz
+# CHECK-SAME: {|}~^?
+# CHECK-SAME: M-^@M-^AM-^BM-^CM-^DM-^EM-^FM-^GM-^HM-^IM-^JM-^KM-^LM-^MM-^NM-^OM-^PM-^QM-^RM-^SM-^TM-^UM-^VM-^WM-^XM-^YM-^Z
+# CHECK-SAME: M-^[M-^\M-^]M-^^M-^_M- M-!M-"M-#M-$M-%M-&M-'M-(M-)M-*M-+M-,M--M-.M-/
+# CHECK-SAME: M-0M-1M-2M-3M-4M-5M-6M-7M-8M-9
+# CHECK-SAME: M-:M-;M-<M-=M->M-?M-@
+# CHECK-SAME: M-AM-BM-CM-DM-EM-FM-GM-HM-IM-JM-KM-LM-MM-NM-OM-PM-QM-RM-SM-TM-UM-VM-WM-XM-YM-Z
+# CHECK-SAME: M-[M-\M-]M-^M-_M-`
+# CHECK-SAME: M-aM-bM-cM-dM-eM-fM-gM-hM-iM-jM-kM-lM-mM-nM-oM-pM-qM-rM-sM-tM-uM-vM-wM-xM-yM-z
+# CHECK-SAME: M-{M-|M-}M-~M-^?
\ No newline at end of file
diff --git a/llvm/utils/lit/tests/Inputs/shtest-cat/cat-e-newline.txt b/llvm/utils/lit/tests/Inputs/shtest-cat/cat-e-newline.txt
new file mode 100644
index 0000000000000..cd97646461376
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-cat/cat-e-newline.txt
@@ -0,0 +1,12 @@
+# RUN: cat -e %S/txt-files/newline | FileCheck %s
+
+# CHECK: new
+# CHECK: $
+# CHECK: $
+# CHECK: $
+# CHECK: $
+# CHECK: $
+# CHECK: $
+# CHECK: $
+# CHECK: $
+# CHECK: line
\ No newline at end of file
diff --git a/llvm/utils/lit/tests/Inputs/shtest-cat/cat-newline.txt b/llvm/utils/lit/tests/Inputs/shtest-cat/cat-newline.txt
new file mode 100644
index 0000000000000..de762dc1d639f
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-cat/cat-newline.txt
@@ -0,0 +1,12 @@
+# RUN: cat %S/txt-files/newline | FileCheck %s
+
+# CHECK: new
+# CHECK-EMPTY:
+# CHECK-EMPTY:
+# CHECK-EMPTY:
+# CHECK-EMPTY:
+# CHECK-EMPTY:
+# CHECK-EMPTY:
+# CHECK-EMPTY:
+# CHECK-EMPTY:
+# CHECK: line
\ No newline at end of file
diff --git a/llvm/utils/lit/tests/Inputs/shtest-cat/cat-v-allchars.txt b/llvm/utils/lit/tests/Inputs/shtest-cat/cat-v-allchars.txt
new file mode 100644
index 0000000000000..70643efd3897a
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-cat/cat-v-allchars.txt
@@ -0,0 +1,34 @@
+# C Program to generate all ASCII characters in allchars.txt
+# #include <stdio.h>
+# int main() {
+# FILE *fptr = fopen("allchars.txt", "w");
+# if(fptr == NULL) {
+# return 1;
+# }
+# for(int i = 0; i < 256; i++) {
+# fprintf(fptr, "%c", i);
+# }
+# fclose(fptr);
+# return 0;
+# }
+
+# RUN: cat -v %S/txt-files/allchars | FileCheck %s
+
+# CHECK: ^@^A^B^C^D^E^F^G^H
+# CHECK: ^K^L^M^N^O^P^Q^R^S^T^U^V^W^X^Y^Z
+# CHECK-SAME: ^[^\^]^^^_
+# CHECK-SAME: !"#$%&'()*+,-./
+# CHECK-SAME: 0123456789
+# CHECK-SAME: :;<=>?@
+# CHECK-SAME: ABCDEFGHIJKLMNOPQRSTUVWXYZ
+# CHECK-SAME: [\]^_`
+# CHECK-SAME: abcdefghijklmnopqrstuvwxyz
+# CHECK-SAME: {|}~^?
+# CHECK-SAME: M-^@M-^AM-^BM-^CM-^DM-^EM-^FM-^GM-^HM-^IM-^JM-^KM-^LM-^MM-^NM-^OM-^PM-^QM-^RM-^SM-^TM-^UM-^VM-^WM-^XM-^YM-^Z
+# CHECK-SAME: M-^[M-^\M-^]M-^^M-^_M- M-!M-"M-#M-$M-%M-&M-'M-(M-)M-*M-+M-,M--M-.M-/
+# CHECK-SAME: M-0M-1M-2M-3M-4M-5M-6M-7M-8M-9
+# CHECK-SAME: M-:M-;M-<M-=M->M-?M-@
+# CHECK-SAME: M-AM-BM-CM-DM-EM-FM-GM-HM-IM-JM-KM-LM-MM-NM-OM-PM-QM-RM-SM-TM-UM-VM-WM-XM-YM-Z
+# CHECK-SAME: M-[M-\M-]M-^M-_M-`
+# CHECK-SAME: M-aM-bM-cM-dM-eM-fM-gM-hM-iM-jM-kM-lM-mM-nM-oM-pM-qM-rM-sM-tM-uM-vM-wM-xM-yM-z
+# CHECK-SAME: M-{M-|M-}M-~M-^?
\ No newline at end of file
diff --git a/llvm/utils/lit/tests/Inputs/shtest-cat/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-cat/lit.cfg
new file mode 100644
index 0000000000000..6b179bf5d4f3b
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-cat/lit.cfg
@@ -0,0 +1,8 @@
+import lit.formats
+
+config.name = "shtest-cat"
+config.suffixes = [".txt"]
+config.test_format = lit.formats.ShTest()
+config.test_source_root = None
+config.test_exec_root = None
+config.substitutions.append(("%{python}", '"%s"' % (sys.executable)))
\ No newline at end of file
diff --git a/llvm/utils/lit/tests/Inputs/shtest-cat/txt-files/allchars b/llvm/utils/lit/tests/Inputs/shtest-cat/txt-files/allchars
new file mode 100644
index 0000000000000..c86626638e0bc
Binary files /dev/null and b/llvm/utils/lit/tests/Inputs/shtest-cat/txt-files/allchars differ
diff --git a/llvm/utils/lit/tests/Inputs/shtest-cat/txt-files/newline b/llvm/utils/lit/tests/Inputs/shtest-cat/txt-files/newline
new file mode 100644
index 0000000000000..6702897186ebc
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-cat/txt-files/newline
@@ -0,0 +1,10 @@
+new
+
+
+
+
+
+
+
+
+line
\ No newline at end of file
diff --git a/llvm/utils/lit/tests/shtest-cat.py b/llvm/utils/lit/tests/shtest-cat.py
new file mode 100644
index 0000000000000..412fccd98c5ac
--- /dev/null
+++ b/llvm/utils/lit/tests/shtest-cat.py
@@ -0,0 +1,32 @@
+# Check the cat command
+
+# RUN: not %{lit} -a -v %{inputs}/shtest-cat \
+# RUN: | FileCheck -match-full-lines %s
+#
+# END.
+
+# CHECK: FAIL: shtest-cat :: cat-e-allchars.txt ({{[^)]*}})
+# CHECK: cat -e {{.+}}/allchars | FileCheck {{.*}}
+# CHECK: # executed command: cat -e {{.*}}
+# CHECK: # | Unsupported: 'cat': option -e not recognized
+# CHECK: # error: command failed with exit status: {{.*}}
+# CHECK: # executed command: FileCheck {{.*}}
+# CHECK: # error: command failed with exit status: {{.*}}
+
+# CHECK: FAIL: shtest-cat :: cat-e-newline.txt ({{[^)]*}})
+# CHECK: cat -e {{.+}}/newline | FileCheck {{.*}}
+# CHECK: # executed command: cat -e {{.*}}
+# CHECK: # | Unsupported: 'cat': option -e not recognized
+# CHECK: # error: command failed with exit status: {{.*}}
+# CHECK: # executed command: FileCheck {{.*}}
+# CHECK: # error: command failed with exit status: {{.*}}
+
+# CHECK: PASS: shtest-cat :: cat-newline.txt ({{[^)]*}})
+# CHECK: cat {{.+}}/newline | FileCheck {{.*}}
+# CHECK: # executed command: cat {{.*}}
+# CHECK: # executed command: FileCheck {{.*}}
+
+# CHECK: PASS: shtest-cat :: cat-v-allchars.txt ({{[^)]*}})
+# CHECK: cat -v {{.+}}/allchars | FileCheck {{.*}}
+# CHECK: # executed command: cat -v {{.*}}
+# CHECK: # executed command: FileCheck {{.*}}
\ No newline at end of file
|
|
CC: @ilovepi @petrhosek |
| @@ -0,0 +1,19 @@ | |||
| # RUN: cat -e %S/txt-files/allchars | FileCheck %s | |||
There was a problem hiding this comment.
A comment at the top of the file describing the purpose of this test would be welcome, especially since its not obvious what all the special character matching is for/doing.
| new | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| line |
There was a problem hiding this comment.
| new | |
| line | |
| first | |
| last |
Maybe this makes more sense?
| @@ -0,0 +1,12 @@ | |||
| # RUN: cat %S/txt-files/newline | FileCheck %s | |||
There was a problem hiding this comment.
Maybe direct this to a file w/ > %t, and then you can assert some things about the output? I was thinking verifying the length and other properties, but you could probably just diff it against the original file.
|
I'd suggest changing the title to |
connieyzhu
changed the title
Tests for the same cat options were initially in different files, but now these tests are condensed into 3 files that contain all tests for each cat option, which are cat, cat -v, and cat -e. Also made some minor syntax changes and added clarifying comments.
| # RUN: cat -e %S/txt-files/allchars | FileCheck %s --check-prefix=ALLCHARS | ||
| # RUN: cat -e %S/txt-files/newline | FileCheck %s --check-prefix=NEWLINE | ||
|
|
||
| # ALLCHARS: ^@^A^B^C^D^E^F^G^H $ |
There was a problem hiding this comment.
| # ALLCHARS: ^@^A^B^C^D^E^F^G^H $ | |
| # ALLCHARS: ^@^A^B^C^D^E^F^G^H | |
| # ALLCHARS-SAME: $ |
May want to exclude chars that we can't output/see in the FileCheck line.{{.*}}$ could also work.
My concern is those just look like whitespace, but are probably something else.
|
|
||
| # RUN: cat -v %S/txt-files/allchars | FileCheck %s --check-prefix=ALLCHARS | ||
| # RUN: cat -v %S/txt-files/newline | FileCheck %s --check-prefix=NEWLINE | ||
|
|
There was a problem hiding this comment.
Comments here, similar to the one for -e would help
| config.test_format = lit.formats.ShTest() | ||
| config.test_source_root = None | ||
| config.test_exec_root = None | ||
| config.substitutions.append(("%{python}", '"%s"' % (sys.executable))) |
There was a problem hiding this comment.
Are you using python? If not, please remove.
More comments describing the purpose of the tests were added for clarification purposes. Some small syntax changes were also made for better readability.
This patch adds another test to check the cat functionality with the allchars text file that includes special/meta characters.
There was a problem hiding this comment.
Just realized that with the FileCheck crash (#101582), I can't check that there is no output for diff (which would be the case if the two files were the same). Is there another way to check output without using FileCheck?
There was a problem hiding this comment.
diff returns an error code when files are different. You shouldn't need to use a check line at all, since the command itself should fail.
Here's an example from LLD's tests, but other places do somthing similar.
llvm-project/lld/test/ELF/stdout.s
Line 10 in 72a514f
llvm/utils/lit/tests/shtest-cat.py
Outdated
| ## Test the cat command | ||
|
|
||
| # RUN: not %{lit} -a -v %{inputs}/shtest-cat \ | ||
| # RUN: | FileCheck -match-full-lines %s --dump-input-context=1000 |
There was a problem hiding this comment.
Why is --dump-input-context=1000 needed?
There was a problem hiding this comment.
Oh, that was just to get more context as I was debugging. I can remove that in my next patch
Made changes to CHECK directives to check output more precisely.
|
✅ With the latest revision this PR passed the Python code formatter. |
|
|
||
|
|
||
|
|
||
| last |
There was a problem hiding this comment.
Is the missing line ending at EOF deliberate?
There was a problem hiding this comment.
It is not, but I believe it does test a special case where a file does not end with a newline. Do you think this is worthwhile to test, or should a newline be added?
| config.suffixes = [".txt"] | ||
| config.test_format = lit.formats.ShTest() | ||
| config.test_source_root = None | ||
| config.test_exec_root = None |
There was a problem hiding this comment.
Please add a new line at EOF, to avoid the git "no new line at end of file" warning.
| @@ -0,0 +1,36 @@ | |||
| ## Tests to check cat -e output | |||
|
|
|||
| # RUN: cat -e %S/txt-files/allchars | FileCheck %s --check-prefix=ALLCHARS | |||
There was a problem hiding this comment.
I think it would be worth in all of these test cases adding --implicit-check-not={{.}} to the FileCheck commands. This will prevent extra output between all the different checks from going unnoticed. You probably should also add --match-full-lines and --strict-whitespace to prevent whitespace being canonicalised by FileCheck. If you do this, you'll need to remove all whitespace from your check lines (between the colon and the actual thing being checked), and make sure hard tabs/spaces only appear where you'd expect them to.
| @@ -0,0 +1,36 @@ | |||
| ## Tests to check cat -e output | |||
There was a problem hiding this comment.
| ## Tests to check cat -e output | |
| ## Tests to check cat -e output. |
Nit here and throughout: comments should end with a ".".
I think it would also be useful to include a brief description of what -e means.
| # ALLCHARS-SAME: M-{M-|M-}M-~M-^? | ||
|
|
||
| ## Test output of -e for new lines | ||
| # NEWLINE: first |
There was a problem hiding this comment.
This should have a $ on the end, yes?
| @@ -0,0 +1,50 @@ | |||
| ## C Program to generate all ASCII characters in allchars | |||
| # #include <stdio.h> | |||
There was a problem hiding this comment.
This code is still part of the comment, so should use ## for the comment markers. That being said, I don't think it's useful. I think instead, just summarise the contents of allchars.txt something like "allchars.txt contains every possible byte value in sequence, and no other characters." This should appear in both test files that use it (so if we drop one, we still have the explanation in the other).
| # return 0; | ||
| # } | ||
|
|
||
| ## Tests to check cat -v output |
There was a problem hiding this comment.
Similar to my comment for the -e case, I think it would be useful to include a brief description of what the -v output actually means.
|
|
||
| ## Tests to check cat -v output | ||
|
|
||
| # RUN: cat -v %S/txt-files/allchars | FileCheck %s --check-prefix=ALLCHARS |
There was a problem hiding this comment.
As above, consider adding --implicit-check-not={{.}}, --strict-whitespace, and --match-full-lines to the FileCheck commands.
llvm/utils/lit/tests/shtest-cat.py
Outdated
| # CHECK: cat {{.+}}/allchars > {{.+}} | ||
| # CHECK-NEXT: # executed command: cat {{.+}}/allchars | ||
| # CHECK: diff {{.+}}/allchars {{.+}} | ||
| # CHECK-NEXT: # executed command: diff {{.+}}/allchars {{.+}} |
| @@ -0,0 +1,16 @@ | |||
| ## Tests to check cat output | |||
|
|
|||
| # RUN: cat %S/txt-files/newline | FileCheck %s --check-prefix=NEWLINE | |||
There was a problem hiding this comment.
Can we simply use diff here instead of FileCheck, like you do below?
This patch removes cat test files created in previous patches in this PR and adds -e tests to valid-shell.txt, which already has existing cat tests.
|
I realized I somehow grepped incorrectly, and there actually already exists tests for
@jh7370 I've added these options to the new |
No problem.
I think the typical LLVM process would be to improve the existing tests first in a PR prior to this one, then base this PR on top of it. You could use a tool like spr to stack your PRs, to aid with this, or just keep them separate then rebase this PR once you land the first one. |
There was a problem hiding this comment.
Picking up on your earlier comment, I think showing the behaviour of cat with a file without a trailing new line is indeed an interesting behaviour to test. It will require a new test input.
Looking more generally at the existing test file, if I'm not mistaken, you should be able to remove the NPLONG-CAT-OUTPUT checks, since they're identical to NP-CAT-OUTPUT. I'd also consider splitting all these cat tests into a separate file (like you had before) for ease of identification in the future. You can keep all the cat tests in the same test file, but I'd suggest it not be mixed with the other test behaviour. I'd do all that in the same prerequisite PR that I mentioned, to avoid conflating new tests with existing tests. I'd even consider two prerequisite PRs: the first to move the tests, and the second to improve them as suggested. This would make reviewing the second of these easier. Up to you though.
| # | ||
| # RUN: cat -e %S/cat_nonprinting.bin \ | ||
| # RUN: | FileCheck --check-prefix=NPE-CAT-OUTPUT --implicit-check-not={{.}} --match-full-lines --strict-whitespace %s | ||
| # NPE-CAT-OUTPUT:^@^A^B^C^D^E^F^G ^H$ |
There was a problem hiding this comment.
Nit, for readability, I might add some spacing as in the suggestion, so that the colons line up.
| # NPE-CAT-OUTPUT:^@^A^B^C^D^E^F^G ^H$ | |
| # NPE-CAT-OUTPUT:^@^A^B^C^D^E^F^G ^H$ |
This patch makes changes to improve syntax in tests and to add more strict checks on cat output. This is a prequisite for #101530.
…file (#102366) This patch separates the lit tests that check for the functionality of lit's built-in cat command into its own test file and folder. This is a prerequisite for llvm/llvm-project#101530.
This patch makes changes to improve syntax in tests and to add more strict checks on cat output. This is a prequisite for llvm/llvm-project#101530.
This patch creates preliminary checks for lit's built-in cat command. These tests verify the cat and cat -v functionality, and display failures with using cat -e which will be implemented in a later patch. This is a prequisite for #102061, and is dependent on #102366.