Web lists-archives.com

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




Phillip Wood <phillip.wood@xxxxxxxxxxxx> writes:

>  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.

> +	NAME="A N Other" &&
> +	EMAIL="a.n.other@xxxxxxxxxxx" &&
> +	{
> +		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.

> +		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@xxxxxxxxxxx" &&
> +	{
> +		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