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

[Charles Bailey <charles@hashpling.org>]

On Wed, May 13, 2009 at 03:35:44PM -0400, Don Slutz wrote:
>  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/