Subject: Re: [PATCH v4 09/11] t0410: test fetching from many promisor remotes

From: SZEDER Gábor <[email-masked]>
Date: Fri, 5 Apr 2019 02:56:24 +0200

On Mon, Apr 01, 2019 at 06:40:43PM +0200, Christian Couder wrote:
> From: Christian Couder <[email-masked]>
> 
> This shows that it is now possible to fetch objects from more
> than one promisor remote, and that fetching from a new
> promisor remote can configure it as one.
> 
> Signed-off-by: Christian Couder <[email-masked]>
> ---
>  t/t0410-partial-clone.sh | 47 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
> index 9266037714..146b0a1e03 100755
> --- a/t/t0410-partial-clone.sh
> +++ b/t/t0410-partial-clone.sh
> @@ -182,8 +182,53 @@ test_expect_success 'fetching of missing objects works with ref-in-want enabled'
>  	grep "git< fetch=.*ref-in-want" trace
>  '
>  
> +test_expect_success 'fetching of missing objects from another promisor remote' '
> +	git clone "file://$(pwd)/server" server2 &&
> +	test_commit -C server2 bar &&
> +	git -C server2 repack -a -d --write-bitmap-index &&
> +	HASH2=$(git -C server2 rev-parse bar) &&
> +
> +	git -C repo remote add server2 "file://$(pwd)/server2" &&
> +	git -C repo config remote.server2.promisor true &&
> +	git -C repo cat-file -p "$HASH2" &&
> +
> +	git -C repo fetch server2 &&
> +	rm -rf repo/.git/objects/* &&
> +	git -C repo cat-file -p "$HASH2" &&
> +
> +	# Ensure that the .promisor file is written, and check that its
> +	# associated packfile contains the object
> +	ls repo/.git/objects/pack/pack-*.promisor >promisorlist &&
> +	test_line_count = 1 promisorlist &&
> +	IDX=$(cat promisorlist | sed "s/promisor$/idx/") &&

You could drop the unnecessary 'cat', 'sed' is capable to open a file
on its own.

> +     git verify-pack --verbose "$IDX" | grep "$HASH2"

Please don't run a git command, especially one with "verify" in its
name, upstream of a pipe, because the pipe will hide the git command's
exit code.

> +'
> +
> +test_expect_success 'fetching of missing objects configures a promisor remote' '
> +	git clone "file://$(pwd)/server" server3 &&
> +	test_commit -C server3 baz &&
> +	git -C server3 repack -a -d --write-bitmap-index &&
> +	HASH3=$(git -C server3 rev-parse baz) &&
> +	git -C server3 config uploadpack.allowfilter 1 &&
> +
> +	rm repo/.git/objects/pack/pack-*.promisor &&
> +
> +	git -C repo remote add server3 "file://$(pwd)/server3" &&
> +	git -C repo fetch --filter="blob:none" server3 $HASH3 &&
> +
> +	test "$(git -C repo config remote.server3.promisor)" = "true" &&

Please use test_cmp_config.

> +	# Ensure that the .promisor file is written, and check that its
> +	# associated packfile contains the object
> +	ls repo/.git/objects/pack/pack-*.promisor >promisorlist &&
> +	test_line_count = 1 promisorlist &&
> +	IDX=$(cat promisorlist | sed "s/promisor$/idx/") &&
> +	git verify-pack --verbose "$IDX" | grep "$HASH3"

Likewise.

> +'
> +
>  test_expect_success 'fetching of missing blobs works' '
> -	rm -rf server repo &&
> +	rm -rf server server2 repo &&
> +	rm -rf server server3 repo &&
>  	test_create_repo server &&
>  	test_commit -C server foo &&
>  	git -C server repack -a -d --write-bitmap-index &&
> -- 
> 2.21.0.203.gd44fa53258
>