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

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

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