git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrei Rybak <rybak.a.v@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Stefan Beller <sbeller@google.com>, Paul Tan <pyokagan@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [RFC] broken test for git am --scissors
Date: Fri, 3 Aug 2018 15:38:03 +0200	[thread overview]
Message-ID: <f91c7393-4f1b-1cf5-b870-f42e9bd18d64@gmail.com> (raw)

I was tweaking is_scissors_line function in mailinfo.c and tried writing
some tests for it.  And discovered that existing test for git am option
--scissors is broken.  I then confirmed that by intentionally breaking
is_scissors_line, like this:

--- 8< ---
Subject: [PATCH 1/2] mailinfo.c: intentionally break is_scissors_line

It seems that tests for "git am" don't actually test the --scissor
option logic.  Break is_scissors_line function by using bogus symbols to
be able to check the tests.

Note that test suite does not pass with this patch applied.  The
expected failure does not happen.
---
 mailinfo.c    | 4 ++--
 t/t4150-am.sh | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/mailinfo.c b/mailinfo.c
index 3281a37d5..7938d85e3 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -682,8 +682,8 @@ static int is_scissors_line(const char *line)
 			perforation++;
 			continue;
 		}
-		if ((!memcmp(c, ">8", 2) || !memcmp(c, "8<", 2) ||
-		     !memcmp(c, ">%", 2) || !memcmp(c, "%<", 2))) {
+		if ((!memcmp(c, ">o", 2) || !memcmp(c, "o<", 2) ||
+		     !memcmp(c, ">/", 2) || !memcmp(c, "/<", 2))) {
 			in_perforation = 1;
 			perforation += 2;
 			scissors += 2;
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 1ebc587f8..59bcb5afd 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -412,7 +412,8 @@ test_expect_success 'am with failing post-applypatch hook' '
 	test_cmp head.expected head.actual
 '
 
-test_expect_success 'am --scissors cuts the message at the scissors line' '
+# Test should fail, but succeeds
+test_expect_failure 'am --scissors cuts the message at the scissors line' '
 	rm -fr .git/rebase-apply &&
 	git reset --hard &&
 	git checkout second &&

--- >8 ---

Here's a proof-of-concept patch for the test, to make it actually fail
when is_scissors_line is broken.  It is the easiest way to do so, that I
could come up with, it is not ready to be applied.  I think the two
tests for --scissors should be rewritten pretty much from scratch, with
more obvious naming of files used.

(I made the changes to files in both tests the same just to be able to
re-use file "no-scissors-patch.eml", it's not relevant to the scissor
line in the commit messages.)

--- 8< ---
Subject: [PATCH 2/2] t4150-am.sh: fix test for --scissors

Test for option --scissors should check that the eml file with a scissor
line inside will be cut up and only the part under the cut will be
turned into commit.

However, the test for --scissors generates eml file without such line.
Fix the test for --scissors option.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t4150-am.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 59bcb5afd..5ad71fe05 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -69,17 +69,22 @@ test_expect_success 'setup: messages' '
 
 	EOF
 
+	cat >new-scissors-msg <<-\EOF &&
+	Subject: Test git-am with scissors line
+
+	This line should be included in the commit message.
+	EOF
+
 	cat >scissors-msg <<-\EOF &&
 	Test git-am with scissors line
 
 	This line should be included in the commit message.
 	EOF
 
-	cat - scissors-msg >no-scissors-msg <<-\EOF &&
+	cat - new-scissors-msg >no-scissors-msg <<-\EOF &&
 	This line should not be included in the commit message with --scissors enabled.
 
 	 - - >8 - - remove everything above this line - - >8 - -
-
 	EOF
 
 	signoff="Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
@@ -148,15 +153,15 @@ test_expect_success setup '
 	} >patch1-hg.eml &&
 
 
-	echo scissors-file >scissors-file &&
-	git add scissors-file &&
+	echo file >file &&
+	git add file &&
 	git commit -F scissors-msg &&
 	git tag scissors &&
 	git format-patch --stdout scissors^ >scissors-patch.eml &&
 	git reset --hard HEAD^ &&
 
-	echo no-scissors-file >no-scissors-file &&
-	git add no-scissors-file &&
+	echo file >file &&
+	git add file &&
 	git commit -F no-scissors-msg &&
 	git tag no-scissors &&
 	git format-patch --stdout no-scissors^ >no-scissors-patch.eml &&
@@ -417,7 +422,7 @@ test_expect_failure 'am --scissors cuts the message at the scissors line' '
 	rm -fr .git/rebase-apply &&
 	git reset --hard &&
 	git checkout second &&
-	git am --scissors scissors-patch.eml &&
+	git am --scissors no-scissors-patch.eml &&
 	test_path_is_missing .git/rebase-apply &&
 	git diff --exit-code scissors &&
 	test_cmp_rev scissors HEAD
--

--- >8 ---

Relevant old threads:

1. https://public-inbox.org/git/1435861000-25278-11-git-send-email-pyokagan@gmail.com
2. https://public-inbox.org/git/1436278114-28057-11-git-send-email-pyokagan@gmail.com

             reply	other threads:[~2018-08-03 13:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-03 13:38 Andrei Rybak [this message]
2018-08-03 22:39 ` [PATCH] t4150: fix broken test for am --scissors Andrei Rybak
2018-08-03 23:04   ` Junio C Hamano
2018-08-04  0:39     ` Andrei Rybak
2018-08-04 18:10   ` [PATCH v2] " Andrei Rybak
2018-08-06  8:58     ` Paul Tan
2018-08-06 15:04       ` Junio C Hamano
2018-08-06 17:42       ` Andrei Rybak
2018-08-07 10:53         ` Paul Tan
2018-08-06 17:49     ` [PATCH v3] " Andrei Rybak
2018-08-06 20:14       ` Junio C Hamano
2018-08-07 11:24         ` Andrei Rybak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f91c7393-4f1b-1cf5-b870-f42e9bd18d64@gmail.com \
    --to=rybak.a.v@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=pyokagan@gmail.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).