Re: [PATCH v6 03/10] replace: add test for --graft

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 7, 2014, 22:16 UTC
Message-ID: <xmqqmwckbyri.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140707063540.3708.9611.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

Show 15 quoted lines

> 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?

Show 9 quoted lines

> +{
> +    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?

Show 14 quoted lines

> +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