From: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
Now that chainlint.pl is functional, take advantage of the existing
chainlint self-tests to validate its operation. (While at it, stop
validating chainlint.sed against the self-tests since it will soon be
retired.)
Due to chainlint.sed implementation limitations leaking into the
self-test "expect" files, a few of them require minor adjustment to make
them compatible with chainlint.pl which does not share those
limitations.
First, because `sed` does not provide any sort of real recursion,
chainlint.sed only emulates recursion into subshells, and each level of
recursion leads to a multiplicative increase in complexity of the `sed`
rules. To avoid substantial complexity, chainlint.sed, therefore, only
emulates subshell recursion one level deep. Any subshell deeper than
that is passed through as-is, which means that &&-chains are not checked
in deeper subshells. chainlint.pl, on the other hand, employs a proper
recursive descent parser, thus checks subshells to any depth and
correctly flags broken &&-chains in deep subshells.
Second, due to sed's line-oriented nature, chainlint.sed, by necessity,
folds multi-line quoted strings into a single line. chainlint.pl, on the
other hand, employs a proper lexical analyzer which preserves quoted
strings as-is, including embedded newlines.
Furthermore, the output of chainlint.sed and chainlint.pl do not match
precisely in terms of whitespace. However, since the purpose of the
self-checks is to verify that the ?!AMP?! annotations are being
correctly added, minor whitespace differences are immaterial. For this
reason, rather than adjusting whitespace in all existing self-test
"expect" files to match the new linter's output, the `check-chainlint`
target ignores whitespace differences. Since `diff -w` is not POSIX,
`check-chainlint` attempts to employ `git diff -w`, and only falls back
to non-POSIX `diff -w` (and `-u`) if `git diff` is not available.
Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
---
t/Makefile | 29 +++++++++++++++----
t/chainlint/block.expect | 2 +-
t/chainlint/here-doc-multi-line-string.expect | 3 +-
t/chainlint/multi-line-string.expect | 11 +++++--
t/chainlint/nested-subshell.expect | 2 +-
t/chainlint/t7900-subtree.expect | 13 +++++++--
6 files changed, 46 insertions(+), 14 deletions(-)
diff --git a/t/Makefile b/t/Makefile
index 1c80c0c79a0..11f276774ea 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -38,7 +38,7 @@ T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
-CHAINLINT = sed -f chainlint.sed
+CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
all: $(DEFAULT_TEST_TARGET)
@@ -73,10 +73,29 @@ clean-chainlint:
check-chainlint:
@mkdir -p '$(CHAINLINTTMP_SQ)' && \
- sed -e '/^# LINT: /d' $(patsubst %,chainlint/%.test,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/tests && \
- sed -e '/^[ ]*$$/d' $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/expect && \
- $(CHAINLINT) '$(CHAINLINTTMP_SQ)'/tests | grep -v '^[ ]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \
- diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
+ for i in $(CHAINLINTTESTS); do \
+ echo "test_expect_success '$$i' '" && \
+ sed -e '/^# LINT: /d' chainlint/$$i.test && \
+ echo "'"; \
+ done >'$(CHAINLINTTMP_SQ)'/tests && \
+ { \
+ echo "# chainlint: $(CHAINLINTTMP_SQ)/tests" && \
+ for i in $(CHAINLINTTESTS); do \
+ echo "# chainlint: $$i" && \
+ sed -e '/^[ ]*$$/d' chainlint/$$i.expect; \
+ done \
+ } >'$(CHAINLINTTMP_SQ)'/expect && \
+ $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \
+ grep -v '^[ ]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \
+ if test -f ../GIT-BUILD-OPTIONS; then \
+ . ../GIT-BUILD-OPTIONS; \
+ fi && \
+ if test -x ../git$$X; then \
+ DIFFW="../git$$X --no-pager diff -w --no-index"; \
+ else \
+ DIFFW="diff -w -u"; \
+ fi && \
+ $$DIFFW '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
test-lint-filenames
diff --git a/t/chainlint/block.expect b/t/chainlint/block.expect
index da60257ebc4..37dbf7d95fa 100644
--- a/t/chainlint/block.expect
+++ b/t/chainlint/block.expect
@@ -1,7 +1,7 @@
(
foo &&
{
- echo a
+ echo a ?!AMP?!
echo b
} &&
bar &&
diff --git a/t/chainlint/here-doc-multi-line-string.expect b/t/chainlint/here-doc-multi-line-string.expect
index 2578191ca8a..be64b26869a 100644
--- a/t/chainlint/here-doc-multi-line-string.expect
+++ b/t/chainlint/here-doc-multi-line-string.expect
@@ -1,4 +1,5 @@
(
- cat <<-TXT && echo "multi-line string" ?!AMP?!
+ cat <<-TXT && echo "multi-line
+ string" ?!AMP?!
bap
)
diff --git a/t/chainlint/multi-line-string.expect b/t/chainlint/multi-line-string.expect
index ab0dadf748e..27ff95218e7 100644
--- a/t/chainlint/multi-line-string.expect
+++ b/t/chainlint/multi-line-string.expect
@@ -1,9 +1,14 @@
(
- x="line 1 line 2 line 3" &&
- y="line 1 line2" ?!AMP?!
+ x="line 1
+ line 2
+ line 3" &&
+ y="line 1
+ line2" ?!AMP?!
foobar
) &&
(
- echo "xyz" "abc def ghi" &&
+ echo "xyz" "abc
+ def
+ ghi" &&
barfoo
)
diff --git a/t/chainlint/nested-subshell.expect b/t/chainlint/nested-subshell.expect
index 41a48adaa2b..02e0a9f1bb5 100644
--- a/t/chainlint/nested-subshell.expect
+++ b/t/chainlint/nested-subshell.expect
@@ -6,7 +6,7 @@
) >file &&
cd foo &&
(
- echo a
+ echo a ?!AMP?!
echo b
) >file
)
diff --git a/t/chainlint/t7900-subtree.expect b/t/chainlint/t7900-subtree.expect
index 1cccc7bf7e1..69167da2f27 100644
--- a/t/chainlint/t7900-subtree.expect
+++ b/t/chainlint/t7900-subtree.expect
@@ -1,10 +1,17 @@
(
- chks="sub1sub2sub3sub4" &&
+ chks="sub1
+sub2
+sub3
+sub4" &&
chks_sub=$(cat <<TXT | sed "s,^,sub dir/,"
) &&
- chkms="main-sub1main-sub2main-sub3main-sub4" &&
+ chkms="main-sub1
+main-sub2
+main-sub3
+main-sub4" &&
chkms_sub=$(cat <<TXT | sed "s,^,sub dir/,"
) &&
subfiles=$(git ls-files) &&
- check_equal "$subfiles" "$chkms$chks"
+ check_equal "$subfiles" "$chkms
+$chks"
)
--
gitgitgadget
