[PATCH] Add test cases for git-am

[PATCH] Add test cases for git-am

[Stephan Beyer]

Add t/t4151-am.sh that does basic testing of git-am functionality,
including:
 * am applies patch correctly
 * am changes committer and keeps author
 * am --signoff adds Signed-off-by: line
 * am stays in branch
 * am --signoff does not add Signed-off-by: line if already there
 * am without --keep removes Re: and [PATCH] stuff
 * am --keep really keeps the subject
 * am -3 falls back to 3-way merge
 * am pauses on conflict
 * am --skip works
 * am --resolved works
 * am takes patches from a Pine mailbox
 * am fails on mail without patch
 * am fails on empty patch

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
Hi,

the reason I added this test is because I'm working on git-sequencer (for
GSoC'08), which functions as a common backend for git-am, git-rebase, ...
And I think this test will make my life easier ;-)

As this is my first test, I hope to get some constructive feedback.

Perhaps it is also useful to swap t4150 and t4151 or to merge them.

Ah, and I omitted tests for --whitespace and some other git-apply-specific
stuff.

Regards,
  Stephan

 t/t4151-am.sh |  223 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 223 insertions(+), 0 deletions(-)
 create mode 100755 t/t4151-am.sh

diff --git a/t/t4151-am.sh b/t/t4151-am.sh
new file mode 100755
index 0000000..df4fb5a
--- /dev/null
+++ b/t/t4151-am.sh
@@ -0,0 +1,223 @@
+#!/bin/sh
+
+test_description='git am running'
+
+. ./test-lib.sh
+
+cat >msg <<EOF
+second
+
+Lorem ipsum dolor sit amet, consectetuer sadipscing elitr, sed diam nonumy
+eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam
+voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita
+kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem
+ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod
+tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At
+vero eos et accusam et justo duo dolores et ea rebum.
+
+	Duis autem vel eum iriure dolor in hendrerit in vulputate velit
+	esse molestie consequat, vel illum dolore eu feugiat nulla facilisis
+	at vero eros et accumsan et iusto odio dignissim qui blandit
+	praesent luptatum zzril delenit augue duis dolore te feugait nulla
+	facilisi.
+
+
+Lorem ipsum dolor sit amet,
+consectetuer adipiscing elit, sed diam nonummy nibh euismod tincidunt ut
+laoreet dolore magna aliquam erat volutpat.
+
+  git
+  ---
+  +++
+
+Ut wisi enim ad minim veniam, quis nostrud exerci tation ullamcorper suscipit
+lobortis nisl ut aliquip ex ea commodo consequat. Duis autem vel eum iriure
+dolor in hendrerit in vulputate velit esse molestie consequat, vel illum
+dolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio
+dignissim qui blandit praesent luptatum zzril delenit augue duis dolore te
+feugait nulla facilisi.
+EOF
+
+cat >failmail <<EOF
+From foo@example.com Fri May 23 10:43:49 2008
+From:	foo@example.com
+To:	bar@example.com
+Subject: Re: [RFC/PATCH] git-foo.sh
+Date:	Fri, 23 May 2008 05:23:42 +0200
+
+Sometimes we have to find out that there's nothing left.
+
+EOF
+
+cat >pine <<EOF
+From MAILER-DAEMON Fri May 23 10:43:49 2008
+Date: 23 May 2008 05:23:42 +0200
+From: Mail System Internal Data <MAILER-DAEMON@example.com>
+Subject: DON'T DELETE THIS MESSAGE -- FOLDER INTERNAL DATA
+Message-ID: <foo-0001@example.com>
+
+This text is part of the internal format of your mail folder, and is not
+a real message.  It is created automatically by the mail system software.
+If deleted, important folder data will be lost, and it will be re-created
+with the data reset to initial values.
+
+EOF
+
+test_expect_success setup '
+	echo hello >file &&
+	git add file &&
+	test_tick &&
+	git commit -m first &&
+	git tag first &&
+	echo world >>file &&
+	git add file &&
+	test_tick &&
+	git commit -s -F msg &&
+	git tag second &&
+	git format-patch --stdout first >patch1 &&
+	tail -n +3 msg >file &&
+	git add file &&
+	test_tick &&
+	git commit -m third &&
+	git tag third &&
+	git format-patch --stdout first >patch2	&&
+	git checkout -b lorem &&
+	tail -n +11 msg >file &&
+	head -n +9 msg >>file &&
+	test_tick &&
+	git commit -a -m "moved stuff" &&
+	echo goodbye >another &&
+	git add another &&
+	test_tick &&
+	git commit -m "added another file" &&
+	git format-patch --stdout master >lorem-move.patch
+'
+
+# reset time
+unset test_tick
+test_tick
+
+test_expect_success 'am applies patch correctly' '
+	git checkout first &&
+	test_tick &&
+	git am <patch1 &&
+	! test -d .dotest &&
+	test -z "$(git diff second)" &&
+	test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
+'
+
+GIT_AUTHOR_NAME="Another Thor"
+GIT_AUTHOR_EMAIL="a.thor@example.com"
+GIT_COMMITTER_NAME="Co M Miter" 
+GIT_COMMITTER_EMAIL="c.miter@example.com"
+export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL
+
+compare () {
+	test "$(git cat-file commit "$2" | grep "^$1 ")" = \
+	     "$(git cat-file commit "$3" | grep "^$1 ")"
+}
+
+test_expect_success 'am changes committer and keeps author' '
+	test_tick &&
+	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_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_expect_success 'am stays in branch' '
+	test "refs/heads/master2" = "$(git symbolic-ref HEAD)"
+'
+
+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," patch3 >patch4
+	git checkout HEAD^ &&
+	git am --signoff patch4 &&
+	test "$(git cat-file commit HEAD | grep -c "^Signed-off-by:")" -eq 1
+'
+
+test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '
+	test "$(git rev-parse HEAD)" = "$(git rev-parse master2)"
+'
+
+test_expect_success 'am --keep really keeps the subject' '
+	git checkout HEAD^ &&
+	git am --keep patch4 &&
+	! test -d .dotest &&
+	git-cat-file commit HEAD |
+		grep -q -F "Re: Re: Re: [PATCH 1/5 v2] third"
+'
+
+test_expect_success 'am -3 falls back to 3-way merge' '
+	git checkout -b lorem2 master2 &&
+	tail -n +3 msg >file &&
+	head -n +9 msg >>file &&
+	git add file &&
+	test_tick &&
+	git commit -m "copied stuff" &&
+	git am -3 lorem-move.patch &&
+	! test -d .dotest &&
+	test -z "$(git diff lorem)"
+'
+
+test_expect_success 'am pauses on conflict' '
+	git checkout lorem2^^ &&
+	! git am lorem-move.patch &&
+	test -d .dotest
+'
+
+test_expect_success 'am --skip works' '
+	git am --skip &&
+	! test -d .dotest &&
+	test -z "$(git diff lorem2^^ -- file)" &&
+	test goodbye = "$(cat another)"
+'
+
+test_expect_success 'am --resolved works' '
+	git checkout lorem2^^ &&
+	! git am lorem-move.patch &&
+	test -d .dotest &&
+	echo resolved >>file &&
+	git add file &&
+	git am --resolved &&
+	! test -d .dotest &&
+	test goodbye = "$(cat another)"
+'
+
+test_expect_success 'am takes patches from a Pine mailbox' '
+	git checkout first &&
+	cat pine patch1 | git am &&
+	! test -d .dotest &&
+	test -z "$(git diff master^..HEAD)"
+'
+
+test_expect_success 'am fails on mail without patch' '
+	! git am <failmail &&
+	rm -r .dotest/
+'
+
+test_expect_success 'am fails on empty patch' '
+	echo "---" >>failmail &&
+	! git am <failmail &&
+	git am --skip &&
+	! test -d .dotest
+'
+
+test_done
-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

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

[Junio C Hamano]

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

> the reason I added this test is because I'm working on git-sequencer (for
> GSoC'08), which functions as a common backend for git-am, git-rebase, ...
> And I think this test will make my life easier ;-)

Absolutely.  Starting by writing a verifiable specification of what you
will be building (aka "tests") is a very good way.

> As this is my first test, I hope to get some constructive feedback.

    Date: Fri, 30 May 2008 16:04:47 +0200
    From: Stephan Beyer <s-beyer@gmx.net>
    To: git@vger.kernel.org
    Cc: gitster@pobox.com
    Subject: [PATCH] Add test cases for git-am
    Message-ID: <20080530140447.GB10514@leksak.fem-net>
    Mail-Followup-To: git@vger.kernel.org, gitster@pobox.com

Please don't do this "Mail-Followup-To:"; it is rude to others, and it is
rude to me.

 * It is rude to others.  When they want to give feedback to _you_
   privately and say "Reply", it will put me (and the list) on "To:"
   header.  They have to be careful and edit their "To:" header.  You
   stole time from them, which otherwise would have been spent on much
   better things, such as actually giving constructive feedbacks.

 * It is rude to me (or whoever you place on your M-F-T).  I filter and
   sort my incoming mails to give precedence to ones that have me on "To:"
   over the ones that have me on "Cc:".  When people want to give feedback
   to _you_ publicly and say "Reply All", it will again put me (and
   perhaps the list) on "To:" header, and I may not be interested in
   suggestions they are giving to _you_.  You stole time from me, which
   otherwise would have been spent on something better, like sitting back
   and sipping my Caipirinha ;-).

> Perhaps it is also useful to swap t4150 and t4151 or to merge them.

Pehaps.  A single test t4150-am.sh might be enough.

> diff --git a/t/t4151-am.sh b/t/t4151-am.sh
> new file mode 100755
> index 0000000..df4fb5a
> --- /dev/null
> +++ b/t/t4151-am.sh
> @@ -0,0 +1,223 @@
> +#!/bin/sh
> +
> +test_description='git am running'
> +
> +. ./test-lib.sh
> +
> +cat >msg <<EOF
> +second
> +
> +Lorem ipsum dolor sit amet, consectetuer sadipscing elitr, sed diam nonumy
> +...
> +feugait nulla facilisi.
> +EOF
> ...
> +test_expect_success setup '
> +	echo hello >file &&
> +	git add file &&
> +	test_tick &&
> +	git commit -m first &&
> +	git tag first &&
> +	echo world >>file &&
> +	git add file &&
> +	test_tick &&
> +	git commit -s -F msg &&
> +	git tag second &&
> +	git format-patch --stdout first >patch1 &&
> +	tail -n +3 msg >file &&

"tail -n 3" you mean?  Same for "head -n <n>" in other places.

> ...
> +test_expect_success 'am changes committer and keeps author' '
> +	test_tick &&
> +	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^
> +'

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.

> +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
> +'

Again, don't you want to check not just "It added something", but "It
added what we expected it to add"?

> ...
> +test_expect_success 'am --keep really keeps the subject' '
> +	git checkout HEAD^ &&
> +	git am --keep patch4 &&
> +	! test -d .dotest &&
> +	git-cat-file commit HEAD |
> +		grep -q -F "Re: Re: Re: [PATCH 1/5 v2] third"
> +'

... in other words, like this one that checks "It left what we expected it
to in the result".

Other than that, I did not think anything obviously wrong with the patch. 

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

[Stephan Beyer]

Hi,

>     Mail-Followup-To: git@vger.kernel.org, gitster@pobox.com
> 
> Please don't do this "Mail-Followup-To:"; it is rude to others, and it is
> rude to me.

Oh, it was not meant to be rude.
In fact, I didn't even knew that my mutt generates it. ;)
So I deactivated it and I hope it helps you to enjoy your Caipirinha.

Btw, I'm confused whether I should use "List reply" (which only
replies to git@vger.kernel.org) or "Reply to all" (which I used now).
While on some other mailinglists it is discouraged to "reply to all",
it seems to be the common case here.

> > Perhaps it is also useful to swap t4150 and t4151 or to merge them.
> 
> Pehaps.  A single test t4150-am.sh might be enough.

Ok.

> > +test_expect_success setup '
> > +	echo hello >file &&
> > +	git add file &&
> > +	test_tick &&
> > +	git commit -m first &&
> > +	git tag first &&
> > +	echo world >>file &&
> > +	git add file &&
> > +	test_tick &&
> > +	git commit -s -F msg &&
> > +	git tag second &&
> > +	git format-patch --stdout first >patch1 &&
> > +	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.

All three cases are POSIX-conform according to the online specs of 
IEEE or OpenGroup, e.g. see
	http://www.opengroup.org/onlinepubs/009695399/utilities/tail.html

> Same for "head -n <n>" in other places.

Here you are right that "head -n 3" equals "head -n +3", since both
result in the first three lines. ("head -n -3" results in all lines
except the last three.)
I kept the '+'-prefix for consistency, but I can remove it if you like.

> > +test_expect_success 'am changes committer and keeps author' '
> > +	test_tick &&
> > +	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^
> > +'
> 
> 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)"
 '
 
 test_expect_success 'am --signoff adds Signed-off-by: line' '
---
Or better ideas? ;)

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

Thank you!
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

Mutt and Mail-Followup-To

[Teemu Likonen]

Stephan Beyer wrote (2008-05-31 04:40 +0200):

> Oh, it was not meant to be rude. In fact, I didn't even knew that my
> mutt generates it. ;) So I deactivated it and I hope it helps you to
> enjoy your Caipirinha.
> 
> Btw, I'm confused whether I should use "List reply" (which only
> replies to git@vger.kernel.org) or "Reply to all" (which I used now).
> While on some other mailinglists it is discouraged to "reply to all",
> it seems to be the common case here.

Mutt's logic is pretty complicated on this. If followup_to=yes, then
Mutt generates M-F-T field and puts all the addresses gathered from To
and CC fields there (minus user's own). In addition to that, if there's
a mailing list address which is in users's "subscribe" list Mutt will
not add users's own address to the M-F-T. If the list address is only in
"lists" list, Mutt will add users's own address. The intention is to
prevent duplicates when user is a subscriber, and to get private replies
if user is not a subscriber.

The logic is so complicated and not very well supported so it doesn't
work well anyway. I think the best thing to do is:

  set followup_to=no
  set honor_followup_to=ask-no

One just has to find out if the list in question favours "replies to
all" (the 'g' key in Mutt) or replies to the list only ('L' in Mutt).

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

[Junio C Hamano]

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?

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

[Stephan Beyer]

Hi,

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

I'm fine with that, but I sometimes wonder if systems that do not like
"tail -n +3" really tend to like a "sed -n -e '3,$p'" :)

Well,
Patch v2 follows. (Let's give git-send-email a chance.)

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

[PATCH v2 1/2] Add test cases for git-am

[Stephan Beyer]

Add t/t4151-am.sh that does basic testing of git-am functionality,
including:
 * am applies patch correctly
 * am changes committer and keeps author
 * am --signoff adds Signed-off-by: line
 * am stays in branch
 * am --signoff does not add Signed-off-by: line if already there
 * am without --keep removes Re: and [PATCH] stuff
 * am --keep really keeps the subject
 * am -3 falls back to 3-way merge
 * am pauses on conflict
 * am --skip works
 * am --resolved works
 * am takes patches from a Pine mailbox
 * am fails on mail without patch
 * am fails on empty patch

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
 t/t4151-am.sh |  226 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 226 insertions(+), 0 deletions(-)
 create mode 100755 t/t4151-am.sh

diff --git a/t/t4151-am.sh b/t/t4151-am.sh
new file mode 100755
index 0000000..ec1b442
--- /dev/null
+++ b/t/t4151-am.sh
@@ -0,0 +1,226 @@
+#!/bin/sh
+
+test_description='git am running'
+
+. ./test-lib.sh
+
+cat >msg <<EOF
+second
+
+Lorem ipsum dolor sit amet, consectetuer sadipscing elitr, sed diam nonumy
+eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam
+voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita
+kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem
+ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod
+tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At
+vero eos et accusam et justo duo dolores et ea rebum.
+
+	Duis autem vel eum iriure dolor in hendrerit in vulputate velit
+	esse molestie consequat, vel illum dolore eu feugiat nulla facilisis
+	at vero eros et accumsan et iusto odio dignissim qui blandit
+	praesent luptatum zzril delenit augue duis dolore te feugait nulla
+	facilisi.
+
+
+Lorem ipsum dolor sit amet,
+consectetuer adipiscing elit, sed diam nonummy nibh euismod tincidunt ut
+laoreet dolore magna aliquam erat volutpat.
+
+  git
+  ---
+  +++
+
+Ut wisi enim ad minim veniam, quis nostrud exerci tation ullamcorper suscipit
+lobortis nisl ut aliquip ex ea commodo consequat. Duis autem vel eum iriure
+dolor in hendrerit in vulputate velit esse molestie consequat, vel illum
+dolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio
+dignissim qui blandit praesent luptatum zzril delenit augue duis dolore te
+feugait nulla facilisi.
+EOF
+
+cat >failmail <<EOF
+From foo@example.com Fri May 23 10:43:49 2008
+From:	foo@example.com
+To:	bar@example.com
+Subject: Re: [RFC/PATCH] git-foo.sh
+Date:	Fri, 23 May 2008 05:23:42 +0200
+
+Sometimes we have to find out that there's nothing left.
+
+EOF
+
+cat >pine <<EOF
+From MAILER-DAEMON Fri May 23 10:43:49 2008
+Date: 23 May 2008 05:23:42 +0200
+From: Mail System Internal Data <MAILER-DAEMON@example.com>
+Subject: DON'T DELETE THIS MESSAGE -- FOLDER INTERNAL DATA
+Message-ID: <foo-0001@example.com>
+
+This text is part of the internal format of your mail folder, and is not
+a real message.  It is created automatically by the mail system software.
+If deleted, important folder data will be lost, and it will be re-created
+with the data reset to initial values.
+
+EOF
+
+echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >expected
+
+test_expect_success setup '
+	echo hello >file &&
+	git add file &&
+	test_tick &&
+	git commit -m first &&
+	git tag first &&
+	echo world >>file &&
+	git add file &&
+	test_tick &&
+	git commit -s -F msg &&
+	git tag second &&
+	git format-patch --stdout first >patch1 &&
+	sed -n -e "3,\$p" msg >file &&
+	git add file &&
+	test_tick &&
+	git commit -m third &&
+	git format-patch --stdout first >patch2	&&
+	git checkout -b lorem &&
+	sed -n -e "11,\$p" msg >file &&
+	head -n 9 msg >>file &&
+	test_tick &&
+	git commit -a -m "moved stuff" &&
+	echo goodbye >another &&
+	git add another &&
+	test_tick &&
+	git commit -m "added another file" &&
+	git format-patch --stdout master >lorem-move.patch
+'
+
+# reset time
+unset test_tick
+test_tick
+
+test_expect_success 'am applies patch correctly' '
+	git checkout first &&
+	test_tick &&
+	git am <patch1 &&
+	! test -d .dotest &&
+	test -z "$(git diff second)" &&
+	test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
+'
+
+GIT_AUTHOR_NAME="Another Thor"
+GIT_AUTHOR_EMAIL="a.thor@example.com"
+GIT_COMMITTER_NAME="Co M Miter" 
+GIT_COMMITTER_EMAIL="c.miter@example.com"
+export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL
+
+compare () {
+	test "$(git cat-file commit "$2" | grep "^$1 ")" = \
+	     "$(git cat-file commit "$3" | grep "^$1 ")"
+}
+
+test_expect_success 'am changes committer and keeps author' '
+	test_tick &&
+	git checkout first &&
+	git am patch2 &&
+	! test -d .dotest &&
+	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^ &&
+	test "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" = \
+	     "$(git log -1 --pretty=format:"%cn <%ce>" HEAD)"
+'
+
+test_expect_success 'am --signoff adds Signed-off-by: line' '
+	git checkout -b master2 first &&
+	git am --signoff <patch2 &&
+	echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >>expected &&
+	git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
+	test_cmp actual expected &&
+	echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >expected &&
+	git cat-file commit HEAD | grep "Signed-off-by:" >actual &&
+	test_cmp actual expected
+'
+
+test_expect_success 'am stays in branch' '
+	test "refs/heads/master2" = "$(git symbolic-ref HEAD)"
+'
+
+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," patch3 >patch4
+	git checkout HEAD^ &&
+	git am --signoff patch4 &&
+	test "$(git cat-file commit HEAD | grep -c "^Signed-off-by:")" -eq 1
+'
+
+test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '
+	test "$(git rev-parse HEAD)" = "$(git rev-parse master2)"
+'
+
+test_expect_success 'am --keep really keeps the subject' '
+	git checkout HEAD^ &&
+	git am --keep patch4 &&
+	! test -d .dotest &&
+	git-cat-file commit HEAD |
+		grep -q -F "Re: Re: Re: [PATCH 1/5 v2] third"
+'
+
+test_expect_success 'am -3 falls back to 3-way merge' '
+	git checkout -b lorem2 master2 &&
+	sed -n -e "3,\$p" msg >file &&
+	head -n 9 msg >>file &&
+	git add file &&
+	test_tick &&
+	git commit -m "copied stuff" &&
+	git am -3 lorem-move.patch &&
+	! test -d .dotest &&
+	test -z "$(git diff lorem)"
+'
+
+test_expect_success 'am pauses on conflict' '
+	git checkout lorem2^^ &&
+	! git am lorem-move.patch &&
+	test -d .dotest
+'
+
+test_expect_success 'am --skip works' '
+	git am --skip &&
+	! test -d .dotest &&
+	test -z "$(git diff lorem2^^ -- file)" &&
+	test goodbye = "$(cat another)"
+'
+
+test_expect_success 'am --resolved works' '
+	git checkout lorem2^^ &&
+	! git am lorem-move.patch &&
+	test -d .dotest &&
+	echo resolved >>file &&
+	git add file &&
+	git am --resolved &&
+	! test -d .dotest &&
+	test goodbye = "$(cat another)"
+'
+
+test_expect_success 'am takes patches from a Pine mailbox' '
+	git checkout first &&
+	cat pine patch1 | git am &&
+	! test -d .dotest &&
+	test -z "$(git diff master^..HEAD)"
+'
+
+test_expect_success 'am fails on mail without patch' '
+	! git am <failmail &&
+	rm -r .dotest/
+'
+
+test_expect_success 'am fails on empty patch' '
+	echo "---" >>failmail &&
+	! git am <failmail &&
+	git am --skip &&
+	! test -d .dotest
+'
+
+test_done
-- 
1.5.5.1

[PATCH v2 2/2] Merge t4150-am-subdir.sh and t4151-am.sh into t4150-am.sh

[Stephan Beyer]

This patch moves the am test cases in t4150-am.sh and the
am subdirectory test cases from t/t4150-am-subdir.sh into
t/4151-am.sh.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
 t/t4150-am-subdir.sh           |   72 ----------------------------------------
 t/{t4151-am.sh => t4150-am.sh} |   34 +++++++++++++++++++
 2 files changed, 34 insertions(+), 72 deletions(-)
 delete mode 100755 t/t4150-am-subdir.sh
 rename t/{t4151-am.sh => t4150-am.sh} (90%)

diff --git a/t/t4150-am-subdir.sh b/t/t4150-am-subdir.sh
deleted file mode 100755
index 52069b4..0000000
--- a/t/t4150-am-subdir.sh
+++ /dev/null
@@ -1,72 +0,0 @@
-#!/bin/sh
-
-test_description='git am running from a subdirectory'
-
-. ./test-lib.sh
-
-test_expect_success setup '
-	echo hello >world &&
-	git add world &&
-	test_tick &&
-	git commit -m initial &&
-	git tag initial &&
-	echo goodbye >world &&
-	git add world &&
-	test_tick &&
-	git commit -m second &&
-	git format-patch --stdout HEAD^ >patchfile &&
-	: >expect
-'
-
-test_expect_success 'am regularly from stdin' '
-	git checkout initial &&
-	git am <patchfile &&
-	git diff master >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success 'am regularly from file' '
-	git checkout initial &&
-	git am patchfile &&
-	git diff master >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success 'am regularly from stdin in subdirectory' '
-	rm -fr subdir &&
-	git checkout initial &&
-	(
-		mkdir -p subdir &&
-		cd subdir &&
-		git am <../patchfile
-	) &&
-	git diff master>actual &&
-	test_cmp expect actual
-'
-
-test_expect_success 'am regularly from file in subdirectory' '
-	rm -fr subdir &&
-	git checkout initial &&
-	(
-		mkdir -p subdir &&
-		cd subdir &&
-		git am ../patchfile
-	) &&
-	git diff master >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success 'am regularly from file in subdirectory with full path' '
-	rm -fr subdir &&
-	git checkout initial &&
-	P=$(pwd) &&
-	(
-		mkdir -p subdir &&
-		cd subdir &&
-		git am "$P/patchfile"
-	) &&
-	git diff master >actual &&
-	test_cmp expect actual
-'
-
-test_done
diff --git a/t/t4151-am.sh b/t/t4150-am.sh
similarity index 90%
rename from t/t4151-am.sh
rename to t/t4150-am.sh
index ec1b442..722ae96 100755
--- a/t/t4151-am.sh
+++ b/t/t4150-am.sh
@@ -223,4 +223,38 @@ test_expect_success 'am fails on empty patch' '
 	! test -d .dotest
 '
 
+test_expect_success 'am works from stdin in subdirectory' '
+	rm -fr subdir &&
+	git checkout first &&
+	(
+		mkdir -p subdir &&
+		cd subdir &&
+		git am <../patch1
+	) &&
+	test -z "$(git diff second)"
+'
+
+test_expect_success 'am works from file (relative path given) in subdirectory' '
+	rm -fr subdir &&
+	git checkout first &&
+	(
+		mkdir -p subdir &&
+		cd subdir &&
+		git am ../patch1
+	) &&
+	test -z "$(git diff second)"
+'
+
+test_expect_success 'am works from file (absolute path given) in subdirectory' '
+	rm -fr subdir &&
+	git checkout first &&
+	P=$(pwd) &&
+	(
+		mkdir -p subdir &&
+		cd subdir &&
+		git am "$P/patch1"
+	) &&
+	test -z "$(git diff second)"
+'
+
 test_done
-- 
1.5.5.1

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

[Junio C Hamano]

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

>> I tend to prefer "sed -n -e '3,$p'" for things like this for portability.
>
> I'm fine with that, but I sometimes wonder if systems that do not like
> "tail -n +3" really tend to like a "sed -n -e '3,$p'" :)

Why?  sed is as old as V7, which does not have "tail -n".

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

[Andreas Ericsson]

Stephan Beyer wrote:
> Hi,
> 
>> "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.
> 
> I'm fine with that, but I sometimes wonder if systems that do not like
> "tail -n +3" really tend to like a "sed -n -e '3,$p'" :)
> 

I have yet to come across a sed which doesn't support -n, the range
specifier and the p command, so yes, "sed -n -e '3,$p'" is very
portable indeed.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231