Re: [PATCH 05/12] t: prepare conditional test execution for `set -e`

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> We have some test in our test suite where we use the pattern of
> `test ... && test_expect_succeess` to conditionally execute a test. The
> problem is that when we decide to not execute the test, we'll indeed
> skip the test, but the overall statement will also be unsuccessful. This
> will become a problem once we enable `set -e`.
>
> Prepare for this future by turning this into a proper conditional, which
> is also a bit easier to read overall.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t4032-diff-inter-hunk-context.sh | 12 +++++++-----
>  t/t7450-bad-git-dotfiles.sh        | 24 +++++++++++++-----------
>  2 files changed, 20 insertions(+), 16 deletions(-)

Unlike the ones that involve $?, I very much prefer the explicit
uses of if/then/else in these changes.  Compared to a potential
alternative to rewrite the existing

	check &&
	test_expect_success ...

into

	! check ||
	test_expect_success ...

it is much more clear to explicitly say

	if check
	then
		test_expect_success ...
	fi

> diff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh
> index c98eb6abb2..2d216fb70f 100755
> --- a/t/t4032-diff-inter-hunk-context.sh
> +++ b/t/t4032-diff-inter-hunk-context.sh
> @@ -40,11 +40,13 @@ t() {
>  		test $(git $cmd $file | grep '^@@ ' | wc -l) = $hunks
>  	"
>  
> -	test -f $expected &&
> -	test_expect_success "$label: check output" "
> -		git $cmd $file | grep -v '^index ' >actual &&
> -		test_cmp $expected actual
> -	"
> +	if test -f $expected
> +	then
> +		test_expect_success "$label: check output" "
> +			git $cmd $file | grep -v '^index ' >actual &&
> +			test_cmp $expected actual
> +		"
> +	fi
>  }
>  
>  cat <<EOF >expected.f1.0.1 || exit 1
> diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
> index f512eed278..8cc86522b2 100755
> --- a/t/t7450-bad-git-dotfiles.sh
> +++ b/t/t7450-bad-git-dotfiles.sh
> @@ -220,17 +220,19 @@ check_dotx_symlink () {
>  		)
>  	'
>  
> -	test -n "$refuse_index" &&
> -	test_expect_success "refuse to load symlinked $name into index ($type)" '
> -		test_must_fail \
> -			git -C $dir \
> -			    -c core.protectntfs \
> -			    -c core.protecthfs \
> -			    read-tree $tree 2>err &&
> -		grep "invalid path.*$name" err &&
> -		git -C $dir ls-files -s >out &&
> -		test_must_be_empty out
> -	'
> +	if test -n "$refuse_index"
> +	then
> +		test_expect_success "refuse to load symlinked $name into index ($type)" '
> +			test_must_fail \
> +				git -C $dir \
> +				    -c core.protectntfs \
> +				    -c core.protecthfs \
> +				    read-tree $tree 2>err &&
> +			grep "invalid path.*$name" err &&
> +			git -C $dir ls-files -s >out &&
> +			test_must_be_empty out
> +		'
> +	fi
>  }
>  
>  check_dotx_symlink gitmodules vanilla .gitmodules