Re: [PATCH 3/6] Fix tests to work with core.autocrlf=true

From: Don Slutz <don.slutz@sierraatlantic.com>
Date: May 14, 2009, 13:49 UTC
Message-ID: <4A0C2180.6030000@SierraAtlantic.com>
In-Reply-To: <20090511222011.GA7609@hashpling.org>

On 5/11/2009 6:20 PM, Charles Bailey wrote:

Show 23 quoted lines

> On Mon, May 11, 2009 at 03:28:58PM -0400, Don Slutz wrote:
>   
>> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
>> index e768c3e..bf39e45 100755
>> --- a/t/t7610-mergetool.sh
>> +++ b/t/t7610-mergetool.sh
>> @@ -45,9 +45,9 @@ test_expect_success 'custom mergetool' '
>>      ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
>>      ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
>>      ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
>> -    test "$(cat file1)" = "master updated" &&
>> -    test "$(cat file2)" = "master new" &&
>> -    test "$(cat subdir/file3)" = "master new sub" &&
>> +    test_cat_eq file1 "master updated" &&
>> +    test_cat_eq file2 "master new" &&
>> +    test_cat_eq subdir/file3 "master new sub" &&
>>      git commit -m "branch1 resolved with mergetool"
>>  '
>>     
>
> This change concerns me. At the moment, the mergetool test assumes
> that globally autocrlf is true and has further tests that attempt to
> verify its behaviour with autocrlf set to true.

Assuming that should have been false and then true for the setting of autocrlf. I would say that this code does not fully check for autocrlf=false. For version 1 of the patch I did not look for other tests that dis not include crlf in their name that had tests for changing autocrlf.

Show 22 quoted lines

>  See the very next
> test:
>
> test_expect_success 'mergetool crlf' '
>     git config core.autocrlf true &&
>     git checkout -b test2 branch1
>     test_must_fail git merge master >/dev/null 2>&1 &&
>     ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
>     ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
>     ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
>     test "$(printf x | cat file1 -)" = "$(printf "master updated\r\nx")" &&
>     test "$(printf x | cat file2 -)" = "$(printf "master new\r\nx")" &&
>     test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\r\nx")" &&
>     git commit -m "branch1 resolved with mergetool - autocrlf" &&
>     git config core.autocrlf false &&
>     git reset --hard
> '
>
> Does the resetting of core.autocrlf to false not break the subsequent
> tests?
>
>

It does cause the subsequent tests to not be tested in autocrlf=true. However I have verified that the sequence:

git config core.autocrlf false && git reset --hard

Does not do what you might expect. A "touch file1" can cause "git diff" to start reporting a difference!

Patch set v2 out soon.
   -Don

__________________________________________________________________________________________________________________ DISCLAIMER:"The information contained in this message and the attachments (if any) may be privileged and confidential and protected from disclosure. You are hereby notified that any unauthorized use, dissemination, distribution or copying of this communication, review, retransmission, or taking of any action based upon this information, by persons or entities other than the intended recipient, is strictly prohibited. If you are not the intended recipient or an employee or agent responsible for delivering this message, and have received this communication in error, please notify us immediately by replying to the message and kindly delete the original message, attachments, if any, and all its copies from your computer system. Thank you for your cooperation." ________________________________________________________________________________________________________________