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

From: Junio C Hamano <[email-masked]>
Date: Sat, 31 May 2008 12:22:01 -0700

Stephan Beyer <[email-masked]> 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 <[email-masked]>" = \
> +	     "$(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 <[email-masked]>$" &&
> +	git cat-file commit HEAD^ | grep -q "^Signed-off-by: Co M Miter <[email-masked]>$" &&
> +	git cat-file commit HEAD^ | grep -q "^Signed-off-by: C O Mitter <[email-masked]>$"
>  '
> ---
>
> 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?