Re: [PATCH v2 01/10] t/test-lib: teach --chain-lint to detect broken &&-chains in subshells

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 30, 2018, 18:13 UTC
Message-ID: <20180730181356.GA156463@aiede.svl.corp.google.com>
In-Reply-To: <20180711064642.6933-2-sunshine@sunshineco.com>

Hi,

Eric Sunshine wrote:

Show 10 quoted lines

> The --chain-lint option detects broken &&-chains by forcing the test to
> exit early (as the very first step) with a sentinel value. If that
> sentinel is the test's overall exit code, then the &&-chain is intact;
> if not, then the chain is broken. Unfortunately, this detection does not
> extend to &&-chains within subshells even when the subshell itself is
> properly linked into the outer &&-chain.
>
> Address this shortcoming by feeding the body of the test to a
> lightweight "linter" which can peer inside subshells and identify broken
> &&-chains by pure textual inspection.

Interesting.

Show 5 quoted lines

>                                        Although the linter does not
> actually parse shell scripts, it has enough knowledge of shell syntax to
> reliably deal with formatting style variations (as evolved over the
> years) and to avoid being fooled by non-shell content (such as inside
> here-docs and multi-line strings).

This is causing contrib/subtree tests to fail for me: running "make -C contrib/subtree test" produces

[...]
	*** t7900-subtree.sh ***
	ok 1 - no merge from non-existent subtree
	ok 2 - no pull from non-existent subtree
	ok 3 - add subproj as subtree into sub dir/ with --prefix
	ok 4 - add subproj as subtree into sub dir/ with --prefix and --message
	ok 5 - add subproj as subtree into sub dir/ with --prefix as -P and --message as -m
	ok 6 - add subproj as subtree into sub dir/ with --squash and --prefix and --message
	ok 7 - merge new subproj history into sub dir/ with --prefix
	ok 8 - merge new subproj history into sub dir/ with --prefix and --message
	ok 9 - merge new subproj history into sub dir/ with --squash and --prefix and --message
	ok 10 - merge the added subproj again, should do nothing
	ok 11 - merge new subproj history into subdir/ with a slash appended to the argument of --prefix
	ok 12 - split requires option --prefix
	ok 13 - split requires path given by option --prefix must exist
	ok 14 - split sub dir/ with --rejoin
	ok 15 - split sub dir/ with --rejoin from scratch
	ok 16 - split sub dir/ with --rejoin and --message
	ok 17 - split "sub dir"/ with --branch
	ok 18 - check hash of split
	ok 19 - split "sub dir"/ with --branch for an existing branch
	ok 20 - split "sub dir"/ with --branch for an incompatible branch
	error: bug in the test script: broken &&-chain or run-away HERE-DOC: 
		subtree_test_create_repo "$subtree_test_count" &&
[...]
		)

	Makefile:44: recipe for target 't7900-subtree.sh' failed

The problematic test code looks like this:

	(
		cd "$subtree_test_count/sub proj" &&
		git fetch .. subproj-br &&
		git merge FETCH_HEAD &&

		chks="sub1
sub2
sub3
sub4" &&
		chks_sub=$(cat <<TXT | sed '\''s,^,sub dir/,'\''
$chks
TXT
) &&
		chkms="main-sub1
main-sub2
main-sub3
main-sub4" &&
		chkms_sub=$(cat <<TXT | sed '\''s,^,sub dir/,'\''
$chkms
TXT
) &&

		subfiles=$(git ls-files) &&
		check_equal "$subfiles" "$chkms
$chks"
	)

Ugly quoting, useless use of "cat", etc, aside, I don't think it's missing any &&. Hints?

Thanks, Jonathan