From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 07 Jul 2014 22:16:33 GMT
Subject: Re: [PATCH v6 03/10] replace: add test for --graft
Message-ID: <xmqqmwckbyri.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140707063540.3708.9611.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  t/t6050-replace.sh | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> index fb07ad2..d80a89e 100755
> --- a/t/t6050-replace.sh
> +++ b/t/t6050-replace.sh
> @@ -18,6 +18,33 @@ add_and_commit_file()
>      git commit --quiet -m "$_file: $_msg"
>  }
>  
> +commit_buffer_contains_parents()

SP before (), perhaps?

> +{
> +    git cat-file commit "$1" >payload &&
> +    sed -n -e '/^$/q' -e '/^parent /p' <payload >actual &&
> +    shift &&
> +    : >expected &&
> +    for _parent
> +    do
> +	echo "parent $_parent" >>expected || return 1
> +    done &&

You can redirect the entire loop to simplify the above 5 lines,
which would read better, no?

	for _parent
        do
		echo "parent $_parent"
	done >expect

> +    test_cmp actual expected

As "test_cmp" normally runs "diff", it is better to compare expect
with actual, not the other way around; running the test verbosely,
i.e. "t6050-replace.sh -v", shows how the actual output differs from
what is expected when diagnosing breakage and would be more useful
that way.

If your goal is to test both the object contents with this code
*and* the overall system behaviour with the "$child^$parent" below,
perhaps they should be in the same "commit_has_parents" function,
without forcing the caller to choose between the two or call both,
no?

> +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 &&
> +	commit_buffer_contains_parents $HASH5 &&
> +	commit_has_parents $HASH5 &&
> +	test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 &&
> +	git replace --force -g $HASH5 $HASH4 $HASH3 &&
> +	commit_buffer_contains_parents $HASH5 $HASH4 $HASH3 &&
> +	commit_has_parents $HASH5 $HASH4 $HASH3 &&
> +	git replace -d $HASH5
> +'
> +
>  test_done