Re: [PATCH v5 2/7] replace: add test for --graft

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 2, 2014, 20:49 UTC
Message-ID: <xmqqy4wbfpux.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140628181117.5687.23923.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

Show 19 quoted lines

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  t/t6050-replace.sh | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> index 68b3cb2..ca45a84 100755
> --- a/t/t6050-replace.sh
> +++ b/t/t6050-replace.sh
> @@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' '
>  	test -z "$(git replace)"
>  '
>  
> +test_expect_success '--graft with and without already replaced object' '
> +	test $(git log --oneline | wc -l) = 7 &&
> +	git replace --graft $HASH5 &&
> +	test $(git log --oneline | wc -l) = 3 &&
> +	git cat-file -p $HASH5 | test_must_fail grep parent &&

Please do not run non-git command under test_must_fail.

Show 8 quoted lines

> +	test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 &&
> +	git replace --force -g $HASH5 $HASH4 $HASH3 &&
> +	git cat-file -p $HASH5 | grep "parent $HASH4" &&
> +	git cat-file -p $HASH5 | grep "parent $HASH3" &&
> +	git replace -d $HASH5
> +'
> +
>  test_done

For all these "git cat-file -p $commit | grep ...", I would think you should add "commit_has_parents" helper function to allow a bit more careful testing. As written, the test will mistake a commit with string "parent $HASHx" anywhere, not in the header part, and the test does not ensure that $HASH{3,4} is the {first,second} parent.

	commit_has_parents $HASH5 $HASH4 $HASH3

would then may be implemented like:

	commit_has_parents () {
		git cat-file commit "$1" >payload &&
                sed -n -e '/^$/q' -e 's/^parent //p' <payload >actual &&
		shift &&
                printf 'parent %s\n' "$@" >expect &&
		test_cmp expect actual
	}