Re: [PATCH] Add test cases for git-am
- From
Stephan Beyer <s-beyer@gmx.net>
- Date
- May 31, 2008, 02:40 UTC
- Message-ID
- <20080531024027.GB5907@leksak.fem-net>
- In-Reply-To
- <7vy75rh25i.fsf@gitster.siamese.dyndns.org>
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.
Show 15 quoted lines
> > +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.
Show 18 quoted lines
> > +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