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@xxxxxxxxx> --- 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@xxxxxxxxx 2. https://public-inbox.org/git/1436278114-28057-11-git-send-email-pyokagan@xxxxxxxxx