git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Don Slutz <Don.Slutz@SierraAtlantic.com>
To: Charles Bailey <charles@hashpling.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 3/6] Fix tests to work with core.autocrlf=true
Date: Thu, 14 May 2009 09:49:52 -0400	[thread overview]
Message-ID: <4A0C2180.6030000@SierraAtlantic.com> (raw)
In-Reply-To: <20090511222011.GA7609@hashpling.org>

On 5/11/2009 6:20 PM, Charles Bailey wrote:
> 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.
>  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." 
________________________________________________________________________________________________________________

  reply	other threads:[~2009-05-14 13:50 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-11 19:28 [PATCH 0/6] Add core.autocrlf=true on cygwin by default during tests Don Slutz
2009-05-11 19:28 ` [PATCH 1/6] " Don Slutz
2009-05-11 19:28   ` [PATCH 2/6] Fix tests to work with core.autocrlf=true Don Slutz
2009-05-11 19:28     ` [PATCH 3/6] " Don Slutz
2009-05-11 19:28       ` [PATCH 4/6] " Don Slutz
2009-05-11 19:29         ` [PATCH 5/6] " Don Slutz
2009-05-11 19:29           ` [PATCH 6/6] Add 'make test-text' Don Slutz
2009-05-11 22:20       ` [PATCH 3/6] Fix tests to work with core.autocrlf=true Charles Bailey
2009-05-14 13:49         ` Don Slutz [this message]
2009-05-11 20:04 ` [PATCH 0/6] Add core.autocrlf=true on cygwin by default during tests Eric Blake
2009-05-12 23:27   ` Junio C Hamano
2009-05-13 17:41     ` Junio C Hamano
2009-05-11 20:54 ` Johannes Schindelin
2009-05-12 18:16   ` Don Slutz
2009-05-13 19:35 ` [PATCH v2 0/7] Add GIT_TEST_AUTO_CRLF environment variable to set core.autocrlf on init Don Slutz
2009-05-13 19:35   ` [PATCH v2 1/7] " Don Slutz
2009-05-13 19:35     ` [PATCH v2 2/7] Add support functions for tests in core.autocrlf=true Don Slutz
2009-05-13 19:35       ` [PATCH v2 3/7] Fix tests to work with core.autocrlf=true -- new functions Don Slutz
2009-05-13 19:35         ` [PATCH v2 4/7] Fix tests to work with core.autocrlf=true -- force false Don Slutz
2009-05-13 19:35           ` [PATCH v2 5/7] Fix tests to work with core.autocrlf=true -- cmp to test_cmp Don Slutz
2009-05-13 19:35             ` [PATCH v2 6/7] Fix tests to work with core.autocrlf=true -- test_cmp to cmp Don Slutz
2009-05-13 19:35               ` [PATCH v2 7/7] Add 'make test-text' core.autocrlf=true Don Slutz
2009-05-14  7:43         ` [PATCH v2 3/7] Fix tests to work with core.autocrlf=true -- new functions Charles Bailey
2009-05-14 14:39           ` Don Slutz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A0C2180.6030000@SierraAtlantic.com \
    --to=don.slutz@sierraatlantic.com \
    --cc=charles@hashpling.org \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).