Re: [PATCH v2] am: fix signoff when other trailers are present

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 8, 2017, 19:37 UTC
Message-ID: <xmqqlgmtc0m2.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170808102533.27516-1-phillip.wood@talktalk.net>

Phillip Wood <phillip.wood@talktalk.net> writes:

Show 17 quoted lines

>  test_expect_success 'am --signoff does not add Signed-off-by: line if already there' '
> -	git format-patch --stdout HEAD^ >patch3 &&
> -	sed -e "/^Subject/ s,\[PATCH,Re: Re: Re: & 1/5 v2] [foo," patch3 >patch4 &&
> -	rm -fr .git/rebase-apply &&
> -	git reset --hard &&
> -	git checkout HEAD^ &&
> -	git am --signoff patch4 &&
> -	git cat-file commit HEAD >actual &&
> -	test $(grep -c "^Signed-off-by:" actual) -eq 1
> +	git format-patch --stdout first >patch3 &&
> +	git reset --hard first &&
> +	git am --signoff <patch3 &&
> +	git log --pretty=%B -2 HEAD >actual &&
> +	test_cmp expected-log actual
> +'
> +
> +test_expect_success 'am --signoff adds Signed-off-by: if another author is preset' '

Present?

I think the motivation for adding this is to make sure that what the previous test checks will be true only when the one we are about to add is already at the end. So perhaps the previous test needs to be retitled from "if already there" to something a bit tighter, e.g. "if already at the end"?

Also, strictly speaking, I think this isn't testing if another author is present---it is specifying what should happen when the last one is not us.

Show 6 quoted lines

> +	NAME="A N Other" &&
> +	EMAIL="a.n.other@example.com" &&
> +	{
> +		printf "third\n\nSigned-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\n" \
> +			"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
> +			"$NAME" "$EMAIL" &&

A "printf" tip: you can do

	printf "third\n\n"
	printf "S-o-b: %s <%s>\n" A B C D

to get two lines of the latter. That would clarify what the next test does which wants to add three of them.

Show 43 quoted lines

> +		cat msg &&
> +		printf "Signed-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\n" \
> +			"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
> +			"$NAME" "$EMAIL"
> +	} >expected-log &&
> +	git reset --hard first &&
> +	GIT_COMMITTER_NAME="$NAME" GIT_COMMITTER_EMAIL="$EMAIL" \
> +		git am --signoff <patch3 &&
> +	git log --pretty=%B -2 HEAD >actual &&
> +	test_cmp expected-log actual
> +'
> +
> +test_expect_success 'am --signoff duplicates Signed-off-by: if it is not the last one' '
> +	NAME="A N Other" &&
> +	EMAIL="a.n.other@example.com" &&
> +	{
> +		printf "third\n\nSigned-off-by: %s <%s>\n\
> +Signed-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\n" \
> +			"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
> +			"$NAME" "$EMAIL" \
> +			"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" &&
> +		cat msg &&
> +		printf "Signed-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\
> +Signed-off-by: %s <%s>\n\n" \
> +			"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
> +			"$NAME" "$EMAIL" \
> +			"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL"
> +	} >expected-log &&
> +	git format-patch --stdout first >patch3 &&
> +	git reset --hard first &&
> +	git am --signoff <patch3 &&
> +	git log --pretty=%B -2 HEAD >actual &&
> +	test_cmp expected-log actual
>  '
>  
>  test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '
> +	git format-patch --stdout HEAD^ >tmp &&
> +	sed -e "/^Subject/ s,\[PATCH,Re: Re: Re: & 1/5 v2] [foo," tmp >patch4 &&
> +	git reset --hard HEAD^ &&
> +	git am <patch4 &&
>  	git rev-parse HEAD >expected &&
>  	git rev-parse master2 >actual &&
>  	test_cmp expected actual