Re: [PATCH 3/3] Adding test for `--keep-cr` for git-am.

[Junio C Hamano <gitster@pobox.com>]

"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:

> This test adds test for git-am dos line endings, the command sequence
> 'git format-patch ... | git am ...' and the configuration variable
> `am.keepcr`.
>
> Signed-off-by: Stefan-W. Hahn <stefan.hahn@s-hahn.de>
> ---
>  t/t4253-am-keep-cr-dos.sh |   68 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 68 insertions(+), 0 deletions(-)
>  create mode 100644 t/t4253-am-keep-cr-dos.sh
>
> diff --git a/t/t4253-am-keep-cr-dos.sh b/t/t4253-am-keep-cr-dos.sh
> new file mode 100644

This should be 100755, if this need to be a new separate test.

> index 0000000..a4f5f80
> --- /dev/null
> +++ b/t/t4253-am-keep-cr-dos.sh
> @@ -0,0 +1,68 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2010 Stefan-W. Hahn
> +#
> +
> +test_description='git-am mbox with dos line ending.
> +
> +'
> +. ./test-lib.sh
> +
> +# Three patches which will be added as files with dos line ending.
> +
> +cat > file1 <<\EOF
> +line 1
> +EOF
> +
> +cat > file2 <<\EOF
> +line 1
> +line 2
> +EOF
> +
> +cat > file3 <<\EOF
> +line 1
> +line 2
> +line 3
> +EOF
> +
> +test_expect_success 'setup repository with dos files' '
> +        append_cr <file1 >file
> +        git add file &&
> +        git commit -m Initial &&
> +        git tag start &&
> +        append_cr <file2 >file
> +        git commit -a -m Second &&
> +        git tag start2 &&
> +        append_cr <file3 >file
> +        git commit -a -m Third &&
> +	git format-patch -k --stdout start.. > format-patch.diff
> +'

Hmm, do I see a mixed indentation here?

> +test_expect_success 'mailsplit format-patch of dos files' '
> +        mkdir split &&
> +        git mailsplit -osplit format-patch.diff &&
> +        cat split/0001 split/0002 > mailsplit.diff &&
> +        test_must_fail test_cmp format-patch.diff mailsplit.diff
> +'
> +
> +test_expect_success 'mailsplit --keep-cr format-patch of dos files' '
> +        mkdir split2 &&
> +        git mailsplit --keep-cr -osplit2 format-patch.diff &&
> +        cat split2/0001 split2/0002 > mailsplit2.diff &&
> +        test_cmp format-patch.diff mailsplit2.diff
> +'

These seem to be mailsplit test; are they necessary?  mailsplit is an
internal implementation detail of am, and we might later want to change
what it does as long as the change does not affect what the calling am
ends up doing, but even with benign change the above two test would fail.

> +test_expect_success 'format-patch with dos files --keep-cr' '
> +        git checkout -b new start &&
> +	git format-patch -k --stdout start..master | git am --keep-cr -k -3 &&

If a long line bothers you, you can split the line at pipe like this:

	git format-patch -k --stdout start..master |
	git am --keep-cr -k -3 &&

Because the shell knows that you haven't finished your sentence yet when
it sees the pipe at the end of line, you do not need a backslash at the
end (the same goes for the && at the end).

> +        git diff master
> +'

"git diff" by default does not report presense or absense of difference
with its exit code.  A traditional way to check this is

	test -z "$(git diff master)"

You can choose to use a more modern

	git diff --exit-code master

> +test_expect_success 'format-patch with dos files config.mailsplit' '
> +        git config am.keepcr 1 &&
> +        git checkout -b new3 start &&
> +	git format-patch -k --stdout start..master | git am -k -3 &&
> +        git diff master
> +'

Two tests are lacking.

 - test that "git am" fails without --keep-cr nor configuration when fed a
   history with CRLF.

 - test that "am.keepcr" can be countermanded per "git am" invocation;

Also I think t4252 needs to be taught about this new option, as I think
you would want the second ('please continue') invocation in this sequence

	$ git am --keep-cr mbox
        ... oops, one does not apply
        ... goes to fix
        $ git am

to keep your CR in the payload.