Re: [PATCH v4 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))"

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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> -	test "a" = "$(cd repo && test-tool path-utils prefix_path prefix "$(pwd)/../repolink/a")"
> +	echo "a" >expect &&
> +	test-tool -C repo path-utils prefix_path prefix "$(cd repo && pwd)/../repolink/a" >actual &&

If we fail to cd to 'repo', "$(cd repo && pwd)/../repolink/a" would
silently expand to nonsense, but presumably "test-tool -C repo"
would fail loudly in such a case, so we should be OK here?

> @@ -28,9 +32,10 @@ test_expect_success 'preparing origin repository' '
>  
>  test_expect_success 'local clone without .git suffix' '
>  	git clone -l -s a b &&
> -	(cd b &&
> -	test "$(git config --bool core.bare)" = false &&
> -	git fetch)
> +	echo false >expect &&
> +	git -C b config --bool core.bare >actual &&
> +	test_cmp expect actual &&
> +	git -C b fetch
>  '

I am not sure if the above with full of "git -C" is strictly an
improvement over

	(
		cd b &&
		echo false >expect &&
		git config --bool core.bare >actual &&
		test_cmp expect actual &&
		git fetch
	)

and even if it were, the reason why it is better would be vastly
different from the reason why it is better that we no longer do
"test $(cmd) = false".  I very much hate the pattern described on
the commit title of this step (which by definition this patch fixes
many instances of).  I.e.

	(cd ...; test <op> $(git ...))

might be something you find that needs improvements, but "don't lose
exit status" ONLY applies to the "test <op> $(git ...)" part, and
the other half, i.e. (cd ...), has nothing to do with "don't lose
exit status" badness.