From: Jaime Soriano Pastor <jsorianopastor@gmail.com>
Date: Thu, 21 Aug 2014 13:51:42 GMT
Subject: Re: [PATCH 3/4] Added tests for the case of merged and unmerged entries for the same file
Message-ID: <CAPuZ2NGUTw8NLTyJSgphh=XBgxy4eTFQLF5yxGD3GgpKbc-Vog@mail.gmail.com>
In-Reply-To: <xmqq8umiub9x.fsf@gitster.dls.corp.google.com>

On Wed, Aug 20, 2014 at 11:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jaime Soriano Pastor <jsorianopastor@gmail.com> writes:
>
>> Signed-off-by: Jaime Soriano Pastor <jsorianopastor@gmail.com>
>> ---
>>  t/t9904-unmerged-file-with-merged-entry.sh | 86 ++++++++++++++++++++++++++++++
>
> Isn't this number already used for another test?  A test on the
> index probably belongs to t2XXX or t3XXX family.
>
Umm, I though this test number was free, I just added it to the last+1
position, if I finally add a test I'll take this into account. Thanks.

>>  1 file changed, 86 insertions(+)
>>  create mode 100755 t/t9904-unmerged-file-with-merged-entry.sh
>>
>> diff --git a/t/t9904-unmerged-file-with-merged-entry.sh b/t/t9904-unmerged-file-with-merged-entry.sh
>> new file mode 100755
>> index 0000000..945bc1c
>> --- /dev/null
>> +++ b/t/t9904-unmerged-file-with-merged-entry.sh
>> @@ -0,0 +1,86 @@
>> +#!/bin/sh
>> +
>> +test_description='Operations with unmerged files with merged entries'
>> +
>> +. ./test-lib.sh
>> +
>> +setup_repository() {
>> +     test_commit A conflict A
>> +     test_commit A conflict2 A2 branchbase
>> +     test_commit B conflict B
>> +     test_commit B conflict2 B2
>> +     git checkout branchbase -b branch1
>> +     test_commit C conflict C
>> +     test_commit C conflict2 C2
>> +     test_commit something otherfile otherfile
>> +}
>
> No error is checked here?
>
This is only a helper function for setup, not a test itself.

>> +setup_stage_state() {
>> +     git checkout -f HEAD
>> +     {
>> +             git ls-files -s conflict conflict2
>> +             git merge master > /dev/null
>> +             git ls-files -s conflict conflict2
>> +     } > index
>
> No error is checked here?
>
Same here.

> Style: no SP between redirection operator and its target, i.e.
>
>         git merge master >/dev/null
>         { ... } >index
>
>> +     cat index | git update-index --index-info
>
> Do not cat a single file into a pipeline, i.e.
>
>         git update-index --index-info <index
>
True :) Thanks.