Re: [PATCH] Add test cases for git-am

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

Stephan Beyer <s-beyer@gmx.net> writes:

>> > +	tail -n +3 msg >file &&
>> 
>> "tail -n 3" you mean?
>
> No :-)
> "tail -n 3" or "tail -n -3" results in the last three lines, but
> "tail -n +3" results in the last lines beginning from the third line.

"git grep 'tail.*+' -- '*.sh'" says that this will the first and only
instance of "tail -n +<number>".  The syntax may be POSIX but not all
/usr/bin/tail unfortunately knows about it.

I tend to prefer "sed -n -e '3,$p'" for things like this for portability.
Incidentally, the only /usr/bin/tail that is incapable of +number I have
access to is so old that it does not even want -n and only wants "tail -3"
or "tail +3"; funnily enough, /usr/bin/head there does accept -n.

>> Hmmm.  Checking for inequality does not feel so robust.  You will allow
>> "am" to record garbage and will not be able to detect a breakage.
>
> Oh, right.
>
> Does this feel better?
> --- a/t/t4151-am.sh
> +++ b/t/t4151-am.sh
> @@ -123,15 +123,13 @@ test_expect_success 'am changes committer and keeps author' '
>  	git checkout first &&
>  	git am patch2 &&
>  	! test -d .dotest &&
> -	test "$(git rev-parse master)" != "$(git rev-parse HEAD)" &&
> -	test "$(git rev-parse master^)" != "$(git rev-parse HEAD^)" &&
>  	test "$(git rev-parse master^^)" = "$(git rev-parse HEAD^^)" &&
>  	test -z "$(git diff master..HEAD)" &&
>  	test -z "$(git diff master^..HEAD^)" &&
>  	compare author master HEAD &&
>  	compare author master^ HEAD^ &&
> -	! compare committer master HEAD &&
> -	! compare committer master^ HEAD^
> +	test "Co M Miter <c.miter@example.com>" = \
> +	     "$(git log -1 --pretty=format:"%cn <%ce>" HEAD)"
>  '

That looks like a more direct approach, doesn't it?

>> Again, don't you want to check not just "It added something", but "It
>> added what we expected it to add"?
>
> Like this?
> ---
>  test_expect_success 'am --signoff adds Signed-off-by: line' '
>  	git checkout -b master2 first &&
>  	git am --signoff <patch2 &&
>  	test "$(git cat-file commit HEAD | grep -c "^Signed-off-by:")" -eq 1 &&
> -	test "$(git cat-file commit HEAD^ | grep -c "^Signed-off-by:")" -eq 2
> +	test "$(git cat-file commit HEAD^ | grep -c "^Signed-off-by:")" -eq 2 &&
> +	git cat-file commit HEAD | grep -q "^Signed-off-by: Co M Miter <c.miter@example.com>$" &&
> +	git cat-file commit HEAD^ | grep -q "^Signed-off-by: Co M Miter <c.miter@example.com>$" &&
> +	git cat-file commit HEAD^ | grep -q "^Signed-off-by: C O Mitter <committer@example.com>$"
>  '
> ---
>
> Mh, I thought it is not bad to keep the -eq checks just to go sure nothing
> is added twice by whatever reason.

Why not sed out all Signed-off-by lines and make sure all of what you
expect to appear do appear in the order you expect them to do?