Re: [RFC/PATCH 2/5] t5801 (remote-helpers): add test to fetch tags

From: Jeff King <peff@peff.net>
Date: Jun 4, 2019, 14:22 UTC
Message-ID: <20190604142233.GB10598@sigill.intra.peff.net>
In-Reply-To: <20190604021330.16130-3-felipe.contreras@gmail.com>

On Mon, Jun 03, 2019 at 09:13:27PM -0500, Felipe Contreras wrote:

Show 6 quoted lines

> This used to work, but commit e198b3a740 broke it.
> 
>   e198b3a740 (fetch: replace string-list used as a look-up table with a hashmap)
> 
> Probably all remote helpers that use the import method are affected, but
> we didn't catch the issue.

Thanks for beefing up the tests. It's rather unfortunate that we missed such a severe regression.

Show 9 quoted lines

> +test_expect_failure 'fetch tag' '
> +	(cd server &&
> +	 git tag v1.0
> +	) &&
> +	(cd local &&
> +	 git fetch
> +	) &&
> +	compare_refs local v1.0 server v1.0
> +'

This single-commands might be more readable using "git -C", but this matches the existing style in the test script, so I'm OK with it either way.

Show 18 quoted lines

> diff --git a/t/t5801/git-remote-testgit b/t/t5801/git-remote-testgit
> index f2b551dfaf..6b9f0b5dc7 100755
> --- a/t/t5801/git-remote-testgit
> +++ b/t/t5801/git-remote-testgit
> @@ -12,9 +12,14 @@ url=$2
>  
>  dir="$GIT_DIR/testgit/$alias"
>  
> -refspec="refs/heads/*:refs/testgit/$alias/heads/*"
> +h_refspec="refs/heads/*:refs/testgit/$alias/heads/*"
> +t_refspec="refs/tags/*:refs/testgit/$alias/tags/*"
>  
> -test -n "$GIT_REMOTE_TESTGIT_NOREFSPEC" && refspec=""
> +if test -n "$GIT_REMOTE_TESTGIT_NOREFSPEC"
> +then
> +	h_refspec=""
> +	t_refspec=""
> +fi

And the simplification from the prior step pays off. Looks good.

The rest of it seems pretty sensible (modulo the caveat that I don't know remote-testgit well enough to detect anything subtle).

-Peff