Subject: Re: [PATCH v2 8/8] tests: use "test_cmp_cmd" in misc tests

From: Junio C Hamano <[email-masked]>
Date: Fri, 02 Dec 2022 11:19:22 +0900

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> Change a few miscellaneous tests to use "test_cmp_cmd" to avoid losing
> the exit code of "git".

The step might have started out to do so, but many of them do not
seem to anymore, perhaps because you allowed the scope of the step
to drift and the focus to be lost while developing more?

>  test_expect_success 'symbolic-ref refuses bare sha1' '
> -	test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD)
> +	rev=$(git rev-parse HEAD) &&
> +	test_must_fail git symbolic-ref HEAD "$rev"

This is not a test_cmp_cmd user.  The update is good.

> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 5841f280fb2..3e59ffd18a0 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -296,9 +296,12 @@ test_expect_success FILEMODE 'stage mode and hunk' '
>  	echo content >>file &&
>  	chmod +x file &&
>  	printf "y\\ny\\n" | git add -p &&
> -	git diff --cached file | grep "new mode" &&
> -	git diff --cached file | grep "+content" &&
> -	test -z "$(git diff file)"
> +	git diff --cached file >out &&
> +	grep "new mode" <out &&
> +	git diff --cached file >out &&
> +	grep "+content" <out &&

No need to run the same "diff --cached file" twice.  No need to
redirect into "grep"; giving the file on the command line is more
natural.

> +	git diff file >out &&
> +	test_must_be_empty out
>  '

Other than that, the above is an improvement, but again, it is not a
test_cmp_cmd user.

> diff --git a/t/t7516-commit-races.sh b/t/t7516-commit-races.sh
> index f2ce14e9071..2d38a16480e 100755
> --- a/t/t7516-commit-races.sh
> +++ b/t/t7516-commit-races.sh
> @@ -10,7 +10,8 @@ test_expect_success 'race to create orphan commit' '
>  	test_must_fail env EDITOR=./hare-editor git commit --allow-empty -m tortoise -e &&
>  	git show -s --pretty=format:%s >subject &&
>  	grep hare subject &&
> -	test -z "$(git show -s --pretty=format:%P)"
> +	git show -s --pretty=format:%P >out &&
> +	test_must_be_empty out
>  '

Likewise.

I guess only half of the tests updated are test_cmp_cmd users, so
this step was about half as big when it was originally written?