git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC] broken test for git am --scissors

From
Andrei Rybak <rybak.a.v@gmail.com>
Date
Aug 3, 2018, 13:38 UTC
Message-ID
<f91c7393-4f1b-1cf5-b870-f42e9bd18d64@gmail.com>

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
Next: Andrei Rybak
Message 1 of 12 in “[RFC] broken test for git am --scissors”
  1. Andrei RybakAug 3, 2018
  2. t4150: fix broken test for am --scissorsAndrei Rybak, Aug 3, 2018
  3. Junio C HamanoAug 3, 2018
  4. Andrei RybakAug 4, 2018
  5. t4150: fix broken test for am --scissorsAndrei Rybak, Aug 4, 2018
  6. Paul TanAug 6, 2018
  7. Junio C HamanoAug 6, 2018
  8. Andrei RybakAug 6, 2018
  9. Paul TanAug 7, 2018
  10. t4150: fix broken test for am --scissorsAndrei Rybak, Aug 6, 2018
  11. Junio C HamanoAug 6, 2018
  12. Andrei RybakAug 7, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.