Skip to content

Commit d00113e

Browse files
sunshinecogitster
authored andcommitted
t/Makefile: apply chainlint.pl to existing self-tests
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 35ebb1e commit d00113e

File tree

6 files changed

+46
-14
lines changed

6 files changed

+46
-14
lines changed

t/Makefile

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
3838
THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
3939
TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
4040
CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
41-
CHAINLINT = sed -f chainlint.sed
41+
CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
4242

4343
all: $(DEFAULT_TEST_TARGET)
4444

@@ -73,10 +73,29 @@ clean-chainlint:
7373

7474
check-chainlint:
7575
@mkdir -p '$(CHAINLINTTMP_SQ)' && \
76-
sed -e '/^# LINT: /d' $(patsubst %,chainlint/%.test,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/tests && \
77-
sed -e '/^[ ]*$$/d' $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/expect && \
78-
$(CHAINLINT) '$(CHAINLINTTMP_SQ)'/tests | grep -v '^[ ]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \
79-
diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
76+
for i in $(CHAINLINTTESTS); do \
77+
echo "test_expect_success '$$i' '" && \
78+
sed -e '/^# LINT: /d' chainlint/$$i.test && \
79+
echo "'"; \
80+
done >'$(CHAINLINTTMP_SQ)'/tests && \
81+
{ \
82+
echo "# chainlint: $(CHAINLINTTMP_SQ)/tests" && \
83+
for i in $(CHAINLINTTESTS); do \
84+
echo "# chainlint: $$i" && \
85+
sed -e '/^[ ]*$$/d' chainlint/$$i.expect; \
86+
done \
87+
} >'$(CHAINLINTTMP_SQ)'/expect && \
88+
$(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \
89+
grep -v '^[ ]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \
90+
if test -f ../GIT-BUILD-OPTIONS; then \
91+
. ../GIT-BUILD-OPTIONS; \
92+
fi && \
93+
if test -x ../git$$X; then \
94+
DIFFW="../git$$X --no-pager diff -w --no-index"; \
95+
else \
96+
DIFFW="diff -w -u"; \
97+
fi && \
98+
$$DIFFW '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
8099

81100
test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
82101
test-lint-filenames

t/chainlint/block.expect

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
(
22
foo &&
33
{
4-
echo a
4+
echo a ?!AMP?!
55
echo b
66
} &&
77
bar &&
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
(
2-
cat <<-TXT && echo "multi-line string" ?!AMP?!
2+
cat <<-TXT && echo "multi-line
3+
string" ?!AMP?!
34
bap
45
)

t/chainlint/multi-line-string.expect

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
(
2-
x="line 1 line 2 line 3" &&
3-
y="line 1 line2" ?!AMP?!
2+
x="line 1
3+
line 2
4+
line 3" &&
5+
y="line 1
6+
line2" ?!AMP?!
47
foobar
58
) &&
69
(
7-
echo "xyz" "abc def ghi" &&
10+
echo "xyz" "abc
11+
def
12+
ghi" &&
813
barfoo
914
)

t/chainlint/nested-subshell.expect

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
) >file &&
77
cd foo &&
88
(
9-
echo a
9+
echo a ?!AMP?!
1010
echo b
1111
) >file
1212
)

t/chainlint/t7900-subtree.expect

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
11
(
2-
chks="sub1sub2sub3sub4" &&
2+
chks="sub1
3+
sub2
4+
sub3
5+
sub4" &&
36
chks_sub=$(cat <<TXT | sed "s,^,sub dir/,"
47
) &&
5-
chkms="main-sub1main-sub2main-sub3main-sub4" &&
8+
chkms="main-sub1
9+
main-sub2
10+
main-sub3
11+
main-sub4" &&
612
chkms_sub=$(cat <<TXT | sed "s,^,sub dir/,"
713
) &&
814
subfiles=$(git ls-files) &&
9-
check_equal "$subfiles" "$chkms$chks"
15+
check_equal "$subfiles" "$chkms
16+
$chks"
1017
)

0 commit comments

Comments
 (0)