Re: [PATCH v2 3/7] Fix tests to work with core.autocrlf=true -- new functions

From: Charles Bailey <charles@hashpling.org>
Date: May 14, 2009, 07:43 UTC
Message-ID: <20090514074303.GA8713@hashpling.org>
In-Reply-To: <1242243348-6690-4-git-send-email-Don.Slutz@SierraAtlantic.com>

On Wed, May 13, 2009 at 03:35:44PM -0400, Don Slutz wrote:

Show 35 quoted lines

>  test_expect_success 'mergetool crlf' '
>      git config core.autocrlf true &&
> -    git checkout -b test2 branch1
> +    rm -f .git/index &&
> +    git reset --hard &&
> +    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 ) &&
> @@ -62,16 +66,35 @@ test_expect_success 'mergetool crlf' '
>      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 $autocrlf &&
> +    rm -f .git/index &&
> +    git reset --hard
> +'
> +
> +test_expect_success 'mergetool lf' '
>      git config core.autocrlf false &&
> +    rm -f .git/index &&
> +    git reset --hard &&
> +    git checkout -b test3 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\nx")" &&
> +    test "$(printf x | cat file2 -)" = "$(printf "master new\nx")" &&
> +    test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\nx")" &&
> +    git commit -m "branch1 resolved with mergetool - autocrlf2" &&
> +    git config core.autocrlf $autocrlf &&
> +    rm -f .git/index &&
>      git reset --hard
>  '

Have I missed some previous recent discussion about this patch series? I know that you referenced that long Aug 2007 thread about autocrlf, but is there some more recent discussion about how the test suite works / should work?

mergetool isn't the prime implementor of autocrlf, but it does have some checks to make sure that it works with autocrlf. My impression - probably incorrect - has been that autocrlf is off for the purposes of building and testing git on all platforms, but that some packages switch it on by default on install for user convenience on platforms where this is appropriate.

Your patch seems to be about allowing the entire test suite to run correctly with the autocrlf in any setting. If this is the case, shouldn't the correct fix be to remove tests that are testing that things work with different settings of autocrlf, because these tests are effectively run by a full test suite run with autocrlf alternatively set anyway?

-- 
Charles Bailey
http://ccgi.hashpling.plus.com/blog/