Re: [PATCH] t5582: remove spurious 'cd "$D"' line

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 23, 2021, 23:32 UTC
Message-ID: <xmqq8s0rpwiw.fsf@gitster.g>
In-Reply-To: <tV8xl8isDPhmGxCNmN06tTwhJTve0PsrkakKwLMcFQJybDZO2SGHHbDLZFrcLp1Yda1_KRygSm7-lVDSZSaG-antdalcPnhSqYqcK5Fpifk=@protonmail.com>

Mickey Endito <mickey.endito.2323@protonmail.com> writes:

> The variable D is never defined in test t5582, more severely the test
> fails if D is defined by something outside the test suite, so remove
> this spurious line.

Wow.  Well spotted.

When D is left unset, we end up executing

	cd "" && ...

and it explains why nobody noticed the breakage for nearly a year since c0192df6 (refspec: add support for negative refspecs, 2020-09-30) was written.

Unlike the apparent copy-and-paste source, this is a more modern script that limits the chdir inside subshells to avoid moving around in the main flow of the test, and the fix proposed here looks the most sensible.

Show 26 quoted lines

> Signed-off-by: Mickey Endito <mickey.endito.2323@protonmail.com>
> ---
> To reproduce a failure do
> D=/some/path/which/does/not/exist t/t5582-fetch-negative-refspec.sh
>
> Note: The variable D seems to be a reminiscent similar to t/t5510-fetch.sh,
> which defines "D=$(pwd)". If you want to adopt that way, then you have
> to code a fix yourself. ;-)
>
>  t/t5582-fetch-negative-refspec.sh | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/t/t5582-fetch-negative-refspec.sh b/t/t5582-fetch-negative-refspec.sh
> index e5d2e79ad3..7a80e47c2b 100755
> --- a/t/t5582-fetch-negative-refspec.sh
> +++ b/t/t5582-fetch-negative-refspec.sh
> @@ -105,7 +105,6 @@ test_expect_success "fetch with negative pattern refspec does not expand prefix"
>  '
>
>  test_expect_success "fetch with negative refspec avoids duplicate conflict" '
> -	cd "$D" &&
>  	(
>  		cd one &&
>  		git branch dups/a &&
> --
> 2.30.2