git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 00/12] Improve git-am test coverage
@ 2015-07-02 18:16 Paul Tan
  2015-07-02 18:16 ` [PATCH 01/12] t4150: am.messageid really adds the message id Paul Tan
                   ` (12 more replies)
  0 siblings, 13 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan

Increase test coverage of git-am.sh to help prevent regressions that could arise
from the rewrite of git-am.sh to C. This patch series, along with
pt/am-foreign, improved test coverage as measured by kcov from 56.5%[1] to
67.3%[2].

No tests for git-am's interactive mode, though, as test_terminal does not seem
to attach a pseudo-tty to stdin(?), thus making git-am's "test -t 0" check fail.

This is part of my GSoC project to rewrite git-am.sh to a C builtin[3].

[1] http://pyokagan.github.io/git/20150430132408-a75942b//kcov-merged/git-am.eb79278e.html
[2] http://pyokagan.github.io/git/20150702173751-2fdae08//kcov-merged/git-am.eb79278e.html
[3] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1


Paul Tan (12):
  t4150: am.messageid really adds the message id
  t4150: am fails if index is dirty
  t4151: am --abort will keep dirty index intact
  t4150: am refuses patches when paused
  t4150: am --resolved fails if index has no changes
  t4150: am --resolved fails if index has unmerged entries
  t4150: am with applypatch-msg hook
  t4150: am with pre-applypatch hook
  t4150: am with post-applypatch hook
  t4150: tests for am --[no-]scissors
  t3418: non-interactive rebase --continue with rerere enabled
  t3901: test git-am encoding conversion

 t/t3418-rebase-continue.sh |  19 ++++
 t/t3901-i18n-patch.sh      |  62 ++++++++++++
 t/t4150-am.sh              | 228 +++++++++++++++++++++++++++++++++++++++++++++
 t/t4151-am-abort.sh        |  15 +++
 4 files changed, 324 insertions(+)

-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 01/12] t4150: am.messageid really adds the message id
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-02 18:41   ` Paolo Bonzini
  2015-07-02 18:16 ` [PATCH 02/12] t4150: am fails if index is dirty Paul Tan
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Paolo Bonzini

Since a078f73 (git-am: add --message-id/--no-message-id, 2014-11-25),
the am.messageid setting determines whether the --message-id option is
set by default.

Add a test for this.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index b822a39..3f54bdf 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -563,6 +563,17 @@ test_expect_success 'am --message-id really adds the message id' '
 	test_cmp expected actual
 '
 
+test_expect_success 'am.messageid really adds the message id' '
+	rm -fr .git/rebase-apply &&
+	git checkout -f HEAD^ &&
+	test_config am.messageid true &&
+	git am patch1.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git cat-file commit HEAD | tail -n1 >actual &&
+	grep Message-Id patch1.eml >expected &&
+	test_cmp expected actual
+'
+
 test_expect_success 'am --message-id -s signs off after the message id' '
 	rm -fr .git/rebase-apply &&
 	git reset --hard &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 02/12] t4150: am fails if index is dirty
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
  2015-07-02 18:16 ` [PATCH 01/12] t4150: am.messageid really adds the message id Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-05 15:38   ` Johannes Schindelin
  2015-07-02 18:16 ` [PATCH 03/12] t4151: am --abort will keep dirty index intact Paul Tan
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
will ensure that the index is clean before applying the patch. This is
to prevent changes unrelated to the patch from being committed.

Add a test for this check.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 3f54bdf..0a19136 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -154,6 +154,17 @@ test_expect_success 'am applies patch correctly' '
 	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
 '
 
+test_expect_success 'am fails if index is dirty' '
+	test_when_finished "rm -fr dirtyfile" &&
+	rm -fr .git/rebase-apply &&
+	git checkout -f first &&
+	echo dirtyfile >dirtyfile &&
+	git add dirtyfile &&
+	test_must_fail git am patch1 &&
+	test_path_is_dir .git/rebase-apply &&
+	test_cmp_rev first HEAD
+'
+
 test_expect_success 'am applies patch e-mail not in a mbox' '
 	rm -fr .git/rebase-apply &&
 	git reset --hard &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 03/12] t4151: am --abort will keep dirty index intact
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
  2015-07-02 18:16 ` [PATCH 01/12] t4150: am.messageid really adds the message id Paul Tan
  2015-07-02 18:16 ` [PATCH 02/12] t4150: am fails if index is dirty Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-02 18:16 ` [PATCH 04/12] t4150: am refuses patches when paused Paul Tan
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure
and warn, 2010-12-21), git-am --abort will not touch the index if on the
previous invocation, git-am failed because the index is dirty. This is
to ensure that the user's modifications to the index are not discarded.

Add a test for this.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4151-am-abort.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 833e7b2..05bdc3e 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -95,6 +95,21 @@ test_expect_success 'am --abort will keep the local commits intact' '
 	test_cmp expect actual
 '
 
+test_expect_success 'am --abort will keep dirty index intact' '
+	git reset --hard initial &&
+	echo dirtyfile >dirtyfile &&
+	cp dirtyfile dirtyfile.expected &&
+	git add dirtyfile &&
+	test_must_fail git am 0001-*.patch &&
+	test_cmp_rev initial HEAD &&
+	test_path_is_file dirtyfile &&
+	test_cmp dirtyfile.expected dirtyfile &&
+	git am --abort &&
+	test_cmp_rev initial HEAD &&
+	test_path_is_file dirtyfile &&
+	test_cmp dirtyfile.expected dirtyfile
+'
+
 test_expect_success 'am -3 stops on conflict on unborn branch' '
 	git checkout -f --orphan orphan &&
 	git reset &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 04/12] t4150: am refuses patches when paused
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (2 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 03/12] t4151: am --abort will keep dirty index intact Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-02 18:16 ` [PATCH 05/12] t4150: am --resolved fails if index has no changes Paul Tan
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since c95b138 (Fix git-am safety checks, 2006-09-15), when there is a
session in progress, git-am will check the command-line arguments and
standard input to ensure that the user does not pass it any patches.

Add a test for this.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 0a19136..e5506f5 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -396,6 +396,20 @@ test_expect_success 'am --abort removes a stray directory' '
 	test_path_is_missing .git/rebase-apply
 '
 
+test_expect_success 'am refuses patches when paused' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout lorem2^^ &&
+
+	test_must_fail git am lorem-move.patch &&
+	test_path_is_dir .git/rebase-apply &&
+	test_cmp_rev lorem2^^ HEAD &&
+
+	test_must_fail git am <lorem-move.patch &&
+	test_path_is_dir .git/rebase-apply &&
+	test_cmp_rev lorem2^^ HEAD
+'
+
 test_expect_success 'am --resolved works' '
 	echo goodbye >expected &&
 	rm -fr .git/rebase-apply &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 05/12] t4150: am --resolved fails if index has no changes
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (3 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 04/12] t4150: am refuses patches when paused Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-02 18:16 ` [PATCH 06/12] t4150: am --resolved fails if index has unmerged entries Paul Tan
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since 6d28644 (git-am: do not allow empty commits by mistake.,
2006-02-23), git-am --resolved will check to see if the index has any
changes to prevent the user from creating an empty commit by mistake.

Add a test for this.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index e5506f5..a2bcffe 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -424,6 +424,18 @@ test_expect_success 'am --resolved works' '
 	test_cmp expected another
 '
 
+test_expect_success 'am --resolved fails if index has no changes' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout lorem2^^ &&
+	test_must_fail git am lorem-move.patch &&
+	test_path_is_dir .git/rebase-apply &&
+	test_cmp_rev lorem2^^ HEAD &&
+	test_must_fail git am --resolved &&
+	test_path_is_dir .git/rebase-apply &&
+	test_cmp_rev lorem2^^ HEAD
+'
+
 test_expect_success 'am takes patches from a Pine mailbox' '
 	rm -fr .git/rebase-apply &&
 	git reset --hard &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 06/12] t4150: am --resolved fails if index has unmerged entries
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (4 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 05/12] t4150: am --resolved fails if index has no changes Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-02 18:16 ` [PATCH 07/12] t4150: am with applypatch-msg hook Paul Tan
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since c1d1128 (git-am --resolved: more usable error message.,
2006-04-28), git-am --resolved will check to see if there are any
unmerged entries, and will error out with a user-friendly error message
if there are.

Add a test for this.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index a2bcffe..05494e9 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -436,6 +436,19 @@ test_expect_success 'am --resolved fails if index has no changes' '
 	test_cmp_rev lorem2^^ HEAD
 '
 
+test_expect_success 'am --resolved fails if index has unmerged entries' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout second &&
+	test_must_fail git am -3 lorem-move.patch &&
+	test_path_is_dir .git/rebase-apply &&
+	test_cmp_rev second HEAD &&
+	test_must_fail git am --resolved >err &&
+	test_path_is_dir .git/rebase-apply &&
+	test_cmp_rev second HEAD &&
+	test_i18ngrep "still have unmerged paths" err
+'
+
 test_expect_success 'am takes patches from a Pine mailbox' '
 	rm -fr .git/rebase-apply &&
 	git reset --hard &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 07/12] t4150: am with applypatch-msg hook
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (5 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 06/12] t4150: am --resolved fails if index has unmerged entries Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-06 17:46   ` Junio C Hamano
  2015-07-02 18:16 ` [PATCH 08/12] t4150: am with pre-applypatch hook Paul Tan
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
will invoke the applypatch-msg hooks just after extracting the patch
message. If the applypatch-msg hook exits with a non-zero status, git-am
abort before even applying the patch to the index.

Add tests for this hook.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 05494e9..24c1b42 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -198,6 +198,45 @@ test_expect_success 'am applies patch e-mail with preceding whitespace' '
 	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
 '
 
+test_expect_success 'am with applypatch-msg hook' '
+	test_when_finished "rm -f .git/hooks/applypatch-msg" &&
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	mkdir -p .git/hooks &&
+	cat >.git/hooks/applypatch-msg <<-\EOF &&
+	#!/bin/sh
+	cat "$1" >actual-msg &&
+	echo hook-message >"$1"
+	EOF
+	chmod +x .git/hooks/applypatch-msg &&
+	git am patch1 &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	echo hook-message >expected &&
+	git log -1 --format=format:%B >actual &&
+	test_cmp expected actual &&
+	git log -1 --format=format:%B second >expected &&
+	test_cmp expected actual-msg
+'
+
+test_expect_success 'am with failing applypatch-msg hook' '
+	test_when_finished "rm -f .git/hooks/applypatch-msg" &&
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	mkdir -p .git/hooks &&
+	cat >.git/hooks/applypatch-msg <<-\EOF &&
+	#!/bin/sh
+	exit 1
+	EOF
+	chmod +x .git/hooks/applypatch-msg &&
+	test_must_fail git am patch1 &&
+	test_path_is_dir .git/rebase-apply &&
+	git diff --exit-code first &&
+	test_cmp_rev first HEAD
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 08/12] t4150: am with pre-applypatch hook
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (6 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 07/12] t4150: am with applypatch-msg hook Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-02 18:16 ` [PATCH 09/12] t4150: am with post-applypatch hook Paul Tan
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sg will invoke the pre-applypatch hook after applying the patch
to the index, but before a commit is made. Should the hook exit with a
non-zero status, git am will exit.

Add tests for this hook.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 24c1b42..dd6fe81 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -237,6 +237,44 @@ test_expect_success 'am with failing applypatch-msg hook' '
 	test_cmp_rev first HEAD
 '
 
+test_expect_success 'am with pre-applypatch hook' '
+	test_when_finished "rm -f .git/hooks/pre-applypatch" &&
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	mkdir -p .git/hooks &&
+	cat >.git/hooks/pre-applypatch <<-\EOF &&
+	#!/bin/sh
+	git diff first >diff.actual
+	exit 0
+	EOF
+	chmod +x .git/hooks/pre-applypatch &&
+	git am patch1 &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	test_cmp_rev second HEAD &&
+	test_cmp_rev second^ HEAD^ &&
+	git diff first..second >diff.expected &&
+	test_cmp diff.expected diff.actual
+'
+
+test_expect_success 'am with failing pre-applypatch hook' '
+	test_when_finished "rm -f .git/hooks/pre-applypatch" &&
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	mkdir -p .git/hooks &&
+	cat >.git/hooks/pre-applypatch <<-\EOF &&
+	#!/bin/sh
+	exit 1
+	EOF
+	chmod +x .git/hooks/pre-applypatch &&
+	test_must_fail git am patch1 &&
+	test_path_is_dir .git/rebase-apply &&
+	git diff --exit-code second &&
+	test_cmp_rev first HEAD
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 09/12] t4150: am with post-applypatch hook
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (7 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 08/12] t4150: am with pre-applypatch hook Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-05 15:58   ` Johannes Schindelin
  2015-07-02 18:16 ` [PATCH 10/12] t4150: tests for am --[no-]scissors Paul Tan
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sh will invoke the post-applypatch hook after the patch is
applied and a commit is made. The exit code of the hook is ignored.

Add tests for this hook.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index dd6fe81..62b678c 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -275,6 +275,48 @@ test_expect_success 'am with failing pre-applypatch hook' '
 	test_cmp_rev first HEAD
 '
 
+test_expect_success 'am with post-applypatch hook' '
+	test_when_finished "rm -f .git/hooks/post-applypatch" &&
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	mkdir -p .git/hooks &&
+	cat >.git/hooks/post-applypatch <<-\EOF &&
+	#!/bin/sh
+	git rev-parse HEAD >head.actual
+	git diff second >diff.actual
+	exit 0
+	EOF
+	chmod +x .git/hooks/post-applypatch &&
+	git am patch1 &&
+	test_path_is_missing .git/rebase-apply &&
+	test_cmp_rev second HEAD &&
+	git rev-parse second >head.expected &&
+	test_cmp head.expected head.actual &&
+	git diff second >diff.expected &&
+	test_cmp diff.expected diff.actual
+'
+
+test_expect_success 'am with failing post-applypatch hook' '
+	test_when_finished "rm -f .git/hooks/post-applypatch" &&
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	mkdir -p .git/hooks &&
+	cat >.git/hooks/post-applypatch <<-\EOF &&
+	#!/bin/sh
+	git rev-parse HEAD >head.actual
+	exit 1
+	EOF
+	chmod +x .git/hooks/post-applypatch &&
+	git am patch1 &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code second &&
+	test_cmp_rev second HEAD &&
+	git rev-parse second >head.expected &&
+	test_cmp head.expected head.actual
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 10/12] t4150: tests for am --[no-]scissors
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (8 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 09/12] t4150: am with post-applypatch hook Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-02 18:16 ` [PATCH 11/12] t3418: non-interactive rebase --continue with rerere enabled Paul Tan
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since 017678b (am/mailinfo: Disable scissors processing by default,
2009-08-26), git-am supported the --[no-]scissors option, passing it to
git-mailinfo.

Add tests to ensure that git-am will pass the --scissors option to
git-mailinfo, and that --no-scissors will override the configuration
setting of mailinfo.scissors.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t4150-am.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 62b678c..94e7c18 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -67,6 +67,19 @@ test_expect_success 'setup: messages' '
 
 	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 &&
+	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>"
 '
 
@@ -105,6 +118,20 @@ test_expect_success setup '
 		git format-patch --stdout first | sed -e "1d"
 	} > patch1-ws.eml &&
 
+	echo scissors-file >scissors-file &&
+	git add scissors-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 &&
+	git commit -F no-scissors-msg &&
+	git tag no-scissors &&
+	git format-patch --stdout no-scissors^ >no-scissors-patch.eml &&
+	git reset --hard HEAD^ &&
+
 	sed -n -e "3,\$p" msg >file &&
 	git add file &&
 	test_tick &&
@@ -317,6 +344,27 @@ 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' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout second &&
+	git am --scissors scissors-patch.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code scissors &&
+	test_cmp_rev scissors HEAD
+'
+
+test_expect_success 'am --no-scissors overrides mailinfo.scissors' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout second &&
+	test_config mailinfo.scissors true &&
+	git am --no-scissors no-scissors-patch.eml &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code no-scissors &&
+	test_cmp_rev no-scissors HEAD
+'
+
 test_expect_success 'setup: new author and committer' '
 	GIT_AUTHOR_NAME="Another Thor" &&
 	GIT_AUTHOR_EMAIL="a.thor@example.com" &&
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 11/12] t3418: non-interactive rebase --continue with rerere enabled
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (9 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 10/12] t4150: tests for am --[no-]scissors Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-02 18:16 ` [PATCH 12/12] t3901: test git-am encoding conversion Paul Tan
  2015-07-03 16:24 ` [PATCH 00/12] Improve git-am test coverage Stefan Beller
  12 siblings, 0 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since 8389b52 (git-rerere: reuse recorded resolve., 2006-01-28), git-am
will call git-rerere to re-use recorded merge conflict resolutions if
any occur in a threeway merge.

Add a test to ensure that git-rerere is called by git-am (which handles
the non-interactive rebase).

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t3418-rebase-continue.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 2680375..4428b90 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -40,6 +40,25 @@ test_expect_success 'non-interactive rebase --continue works with touched file'
 	git rebase --continue
 '
 
+test_expect_success 'non-interactive rebase --continue with rerere enabled' '
+	test_config rerere.enabled true &&
+	test_when_finished "test_might_fail git rebase --abort" &&
+	git reset --hard commit-new-file-F2-on-topic-branch &&
+	git checkout master &&
+	rm -fr .git/rebase-* &&
+
+	test_must_fail git rebase --onto master master topic &&
+	echo "Resolved" >F2 &&
+	git add F2 &&
+	cp F2 F2.expected &&
+	git rebase --continue &&
+
+	git reset --hard commit-new-file-F2-on-topic-branch &&
+	git checkout master &&
+	test_must_fail git rebase --onto master master topic &&
+	test_cmp F2.expected F2
+'
+
 test_expect_success 'rebase --continue can not be used with other options' '
 	test_must_fail git rebase -v --continue &&
 	test_must_fail git rebase --continue -v
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 12/12] t3901: test git-am encoding conversion
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (10 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 11/12] t3418: non-interactive rebase --continue with rerere enabled Paul Tan
@ 2015-07-02 18:16 ` Paul Tan
  2015-07-08 20:44   ` Johannes Sixt
  2015-07-03 16:24 ` [PATCH 00/12] Improve git-am test coverage Stefan Beller
  12 siblings, 1 reply; 26+ messages in thread
From: Paul Tan @ 2015-07-02 18:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, Junio C Hamano

Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
supported the --utf8 and --no-utf8 options, and if set, would pass the
-u flag and the -k flag respectively.

git mailinfo -u will re-code the commit log message and authorship info
in the charset specified by i18n.commitencoding setting, while
git mailinfo -n will disable the re-coding.

Since d84029b (--utf8 is now default for 'git-am', 2007-01-08), --utf8
is set by default in git-am.

Add various encoding conversion tests to t3901 to test git-mailinfo's
encoding conversion. In addition, add a test for --no-utf8 to check that
no encoding conversion will occur if that option is set.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 t/t3901-i18n-patch.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
index 75cf3ff..b49bdb7 100755
--- a/t/t3901-i18n-patch.sh
+++ b/t/t3901-i18n-patch.sh
@@ -251,4 +251,66 @@ test_expect_success 'rebase --merge (L/U)' '
 	check_encoding 2 8859
 '
 
+test_expect_success 'am (U/U)' '
+	# Apply UTF-8 patches with UTF-8 commitencoding
+	git config i18n.commitencoding UTF-8 &&
+	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+
+	git reset --hard master &&
+	git am out-u1 out-u2 &&
+
+	check_encoding 2
+'
+
+test_expect_success 'am (L/L)' '
+	# Apply ISO-8859-1 patches with ISO-8859-1 commitencoding
+	git config i18n.commitencoding ISO8859-1 &&
+	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+
+	git reset --hard master &&
+	git am out-l1 out-l2 &&
+
+	check_encoding 2 8859
+'
+
+test_expect_success 'am (U/L)' '
+	# Apply ISO-8859-1 patches with UTF-8 commitencoding
+	git config i18n.commitencoding UTF-8 &&
+	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+	git reset --hard master &&
+
+	# am specifies --utf8 by default.
+	git am out-l1 out-l2 &&
+
+	check_encoding 2
+'
+
+test_expect_success 'am --no-utf8 (U/L)' '
+	# Apply ISO-8859-1 patches with UTF-8 commitencoding
+	git config i18n.commitencoding UTF-8 &&
+	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
+
+	git reset --hard master &&
+	git am --no-utf8 out-l1 out-l2 2>err &&
+
+	# commit-tree will warn that the commit message does not contain valid UTF-8
+	# as mailinfo did not convert it
+	grep "did not conform" err &&
+
+	check_encoding 2
+'
+
+test_expect_success !MINGW 'am (L/U)' '
+	# Apply UTF-8 patches with ISO-8859-1 commitencoding
+	git config i18n.commitencoding ISO8859-1 &&
+	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
+
+	git reset --hard master &&
+	# mailinfo will re-code the commit message to the charset specified by
+	# i18n.commitencoding
+	git am out-u1 out-u2 &&
+
+	check_encoding 2 8859
+'
+
 test_done
-- 
2.5.0.rc1.81.gfe77482

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 01/12] t4150: am.messageid really adds the message id
  2015-07-02 18:16 ` [PATCH 01/12] t4150: am.messageid really adds the message id Paul Tan
@ 2015-07-02 18:41   ` Paolo Bonzini
  2015-07-06 17:38     ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2015-07-02 18:41 UTC (permalink / raw)
  To: Paul Tan, git; +Cc: Johannes Schindelin, Stefan Beller



On 02/07/2015 20:16, Paul Tan wrote:
> Since a078f73 (git-am: add --message-id/--no-message-id, 2014-11-25),
> the am.messageid setting determines whether the --message-id option is
> set by default.
> 
> Add a test for this.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>  t/t4150-am.sh | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
> index b822a39..3f54bdf 100755
> --- a/t/t4150-am.sh
> +++ b/t/t4150-am.sh
> @@ -563,6 +563,17 @@ test_expect_success 'am --message-id really adds the message id' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'am.messageid really adds the message id' '
> +	rm -fr .git/rebase-apply &&
> +	git checkout -f HEAD^ &&
> +	test_config am.messageid true &&
> +	git am patch1.eml &&
> +	test_path_is_missing .git/rebase-apply &&
> +	git cat-file commit HEAD | tail -n1 >actual &&
> +	grep Message-Id patch1.eml >expected &&
> +	test_cmp expected actual
> +'
> +
>  test_expect_success 'am --message-id -s signs off after the message id' '
>  	rm -fr .git/rebase-apply &&
>  	git reset --hard &&
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 00/12] Improve git-am test coverage
  2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
                   ` (11 preceding siblings ...)
  2015-07-02 18:16 ` [PATCH 12/12] t3901: test git-am encoding conversion Paul Tan
@ 2015-07-03 16:24 ` Stefan Beller
  2015-07-05 16:02   ` Johannes Schindelin
  12 siblings, 1 reply; 26+ messages in thread
From: Stefan Beller @ 2015-07-03 16:24 UTC (permalink / raw)
  To: Paul Tan; +Cc: git@vger.kernel.org, Johannes Schindelin

On Thu, Jul 2, 2015 at 11:16 AM, Paul Tan <pyokagan@gmail.com> wrote:
> Increase test coverage of git-am.sh to help prevent regressions that could arise
> from the rewrite of git-am.sh to C. This patch series, along with
> pt/am-foreign, improved test coverage as measured by kcov from 56.5%[1] to
> 67.3%[2].
>
> No tests for git-am's interactive mode, though, as test_terminal does not seem
> to attach a pseudo-tty to stdin(?), thus making git-am's "test -t 0" check fail.
>
> This is part of my GSoC project to rewrite git-am.sh to a C builtin[3].

The whole series looks good to me.

>
> [1] http://pyokagan.github.io/git/20150430132408-a75942b//kcov-merged/git-am.eb79278e.html
> [2] http://pyokagan.github.io/git/20150702173751-2fdae08//kcov-merged/git-am.eb79278e.html
> [3] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1
>
>
> Paul Tan (12):
>   t4150: am.messageid really adds the message id
>   t4150: am fails if index is dirty
>   t4151: am --abort will keep dirty index intact
>   t4150: am refuses patches when paused
>   t4150: am --resolved fails if index has no changes
>   t4150: am --resolved fails if index has unmerged entries
>   t4150: am with applypatch-msg hook
>   t4150: am with pre-applypatch hook
>   t4150: am with post-applypatch hook
>   t4150: tests for am --[no-]scissors
>   t3418: non-interactive rebase --continue with rerere enabled
>   t3901: test git-am encoding conversion
>
>  t/t3418-rebase-continue.sh |  19 ++++
>  t/t3901-i18n-patch.sh      |  62 ++++++++++++
>  t/t4150-am.sh              | 228 +++++++++++++++++++++++++++++++++++++++++++++
>  t/t4151-am-abort.sh        |  15 +++
>  4 files changed, 324 insertions(+)
>
> --
> 2.5.0.rc1.81.gfe77482
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 02/12] t4150: am fails if index is dirty
  2015-07-02 18:16 ` [PATCH 02/12] t4150: am fails if index is dirty Paul Tan
@ 2015-07-05 15:38   ` Johannes Schindelin
  2015-07-07  6:35     ` Paul Tan
  0 siblings, 1 reply; 26+ messages in thread
From: Johannes Schindelin @ 2015-07-05 15:38 UTC (permalink / raw)
  To: Paul Tan; +Cc: git, Stefan Beller, Junio C Hamano

Hi Paul,

On 2015-07-02 20:16, Paul Tan wrote:

> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
> index 3f54bdf..0a19136 100755
> --- a/t/t4150-am.sh
> +++ b/t/t4150-am.sh
> @@ -154,6 +154,17 @@ test_expect_success 'am applies patch correctly' '
>  	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
>  '
>  
> +test_expect_success 'am fails if index is dirty' '
> +	test_when_finished "rm -fr dirtyfile" &&

Seeing as you `git add` that file further down, how about `git rm -f dirtfile` here?

> +	rm -fr .git/rebase-apply &&
> +	git checkout -f first &&
> +	echo dirtyfile >dirtyfile &&
> +	git add dirtyfile &&
> +	test_must_fail git am patch1 &&
> +	test_path_is_dir .git/rebase-apply &&
> +	test_cmp_rev first HEAD

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 09/12] t4150: am with post-applypatch hook
  2015-07-02 18:16 ` [PATCH 09/12] t4150: am with post-applypatch hook Paul Tan
@ 2015-07-05 15:58   ` Johannes Schindelin
  2015-07-07  6:47     ` Paul Tan
  0 siblings, 1 reply; 26+ messages in thread
From: Johannes Schindelin @ 2015-07-05 15:58 UTC (permalink / raw)
  To: Paul Tan; +Cc: git, Stefan Beller, Junio C Hamano

Hi Paul,

On 2015-07-02 20:16, Paul Tan wrote:

> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
> index dd6fe81..62b678c 100755
> --- a/t/t4150-am.sh
> +++ b/t/t4150-am.sh
> @@ -275,6 +275,48 @@ test_expect_success 'am with failing pre-applypatch hook' '
>  	test_cmp_rev first HEAD
>  '
>  
> +test_expect_success 'am with post-applypatch hook' '
> +	test_when_finished "rm -f .git/hooks/post-applypatch" &&
> +	rm -fr .git/rebase-apply &&
> +	git reset --hard &&
> +	git checkout first &&
> +	mkdir -p .git/hooks &&
> +	cat >.git/hooks/post-applypatch <<-\EOF &&
> +	#!/bin/sh
> +	git rev-parse HEAD >head.actual
> +	git diff second >diff.actual
> +	exit 0
> +	EOF
> +	chmod +x .git/hooks/post-applypatch &&
> +	git am patch1 &&
> +	test_path_is_missing .git/rebase-apply &&
> +	test_cmp_rev second HEAD &&
> +	git rev-parse second >head.expected &&
> +	test_cmp head.expected head.actual &&
> +	git diff second >diff.expected &&
> +	test_cmp diff.expected diff.actual
> +'
> +
> +test_expect_success 'am with failing post-applypatch hook' '
> +	test_when_finished "rm -f .git/hooks/post-applypatch" &&
> +	rm -fr .git/rebase-apply &&
> +	git reset --hard &&
> +	git checkout first &&
> +	mkdir -p .git/hooks &&
> +	cat >.git/hooks/post-applypatch <<-\EOF &&
> +	#!/bin/sh
> +	git rev-parse HEAD >head.actual
> +	exit 1
> +	EOF
> +	chmod +x .git/hooks/post-applypatch &&
> +	git am patch1 &&
> +	test_path_is_missing .git/rebase-apply &&
> +	git diff --exit-code second &&
> +	test_cmp_rev second HEAD &&
> +	git rev-parse second >head.expected &&
> +	test_cmp head.expected head.actual
> +'

These 2 tests as well as the previous patches look to me as if they could be refactored (the paradigm is the same: add a certain hook after resetting and then apply the patch, verify that the hook ran/failed)... do you think there is a chance for that?

Just to make sure: I think the patch series looks good, and if it would be too cumbersome (or just not feasible) to simplify those test cases, I am totally fine with them as-are.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 00/12] Improve git-am test coverage
  2015-07-03 16:24 ` [PATCH 00/12] Improve git-am test coverage Stefan Beller
@ 2015-07-05 16:02   ` Johannes Schindelin
  2015-07-06 20:42     ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Johannes Schindelin @ 2015-07-05 16:02 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Paul Tan, git

Hi,

On 2015-07-03 18:24, Stefan Beller wrote:
> On Thu, Jul 2, 2015 at 11:16 AM, Paul Tan <pyokagan@gmail.com> wrote:
>> Increase test coverage of git-am.sh to help prevent regressions that could arise
>> from the rewrite of git-am.sh to C. This patch series, along with
>> pt/am-foreign, improved test coverage as measured by kcov from 56.5%[1] to
>> 67.3%[2].
>>
>> No tests for git-am's interactive mode, though, as test_terminal does not seem
>> to attach a pseudo-tty to stdin(?), thus making git-am's "test -t 0" check fail.
>>
>> This is part of my GSoC project to rewrite git-am.sh to a C builtin[3].
> 
> The whole series looks good to me.

I concur (my two comments really are minor nit picks).

Thanks,
Dscho

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 01/12] t4150: am.messageid really adds the message id
  2015-07-02 18:41   ` Paolo Bonzini
@ 2015-07-06 17:38     ` Junio C Hamano
  0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2015-07-06 17:38 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Paul Tan, git, Johannes Schindelin, Stefan Beller

Thanks, both.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 07/12] t4150: am with applypatch-msg hook
  2015-07-02 18:16 ` [PATCH 07/12] t4150: am with applypatch-msg hook Paul Tan
@ 2015-07-06 17:46   ` Junio C Hamano
  0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2015-07-06 17:46 UTC (permalink / raw)
  To: Paul Tan; +Cc: git, Johannes Schindelin, Stefan Beller

Paul Tan <pyokagan@gmail.com> writes:

> +test_expect_success 'am with applypatch-msg hook' '
> +	test_when_finished "rm -f .git/hooks/applypatch-msg" &&
> +	rm -fr .git/rebase-apply &&
> +	git reset --hard &&
> +	git checkout first &&
> +	mkdir -p .git/hooks &&
> +	cat >.git/hooks/applypatch-msg <<-\EOF &&
> +	#!/bin/sh
> +	cat "$1" >actual-msg &&
> +	echo hook-message >"$1"
> +	EOF
> +	chmod +x .git/hooks/applypatch-msg &&

This (and the other one below) looks like a good candidate for the
write_script helper.

> +	git am patch1 &&
> +	test_path_is_missing .git/rebase-apply &&
> +	git diff --exit-code second &&
> +	echo hook-message >expected &&
> +	git log -1 --format=format:%B >actual &&
> +	test_cmp expected actual &&
> +	git log -1 --format=format:%B second >expected &&
> +	test_cmp expected actual-msg
> +'
> +
> +test_expect_success 'am with failing applypatch-msg hook' '
> +	test_when_finished "rm -f .git/hooks/applypatch-msg" &&
> +	rm -fr .git/rebase-apply &&
> +	git reset --hard &&
> +	git checkout first &&
> +	mkdir -p .git/hooks &&
> +	cat >.git/hooks/applypatch-msg <<-\EOF &&
> +	#!/bin/sh
> +	exit 1
> +	EOF
> +	chmod +x .git/hooks/applypatch-msg &&
> +	test_must_fail git am patch1 &&
> +	test_path_is_dir .git/rebase-apply &&
> +	git diff --exit-code first &&
> +	test_cmp_rev first HEAD
> +'
> +
>  test_expect_success 'setup: new author and committer' '
>  	GIT_AUTHOR_NAME="Another Thor" &&
>  	GIT_AUTHOR_EMAIL="a.thor@example.com" &&

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 00/12] Improve git-am test coverage
  2015-07-05 16:02   ` Johannes Schindelin
@ 2015-07-06 20:42     ` Junio C Hamano
  0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2015-07-06 20:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Stefan Beller, Paul Tan, git

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> On 2015-07-03 18:24, Stefan Beller wrote:
>> On Thu, Jul 2, 2015 at 11:16 AM, Paul Tan <pyokagan@gmail.com> wrote:
>>> Increase test coverage of git-am.sh to help prevent regressions
>>> that could arise
>>> from the rewrite of git-am.sh to C. This patch series, along with
>>> pt/am-foreign, improved test coverage as measured by kcov from 56.5%[1] to
>>> 67.3%[2].
>>>
>>> No tests for git-am's interactive mode, though, as test_terminal does not seem
>>> to attach a pseudo-tty to stdin(?), thus making git-am's "test -t
>>> 0" check fail.
>>>
>>> This is part of my GSoC project to rewrite git-am.sh to a C builtin[3].
>> 
>> The whole series looks good to me.
>
> I concur (my two comments really are minor nit picks).
>
> Thanks,
> Dscho

Yeah, looked more-or-less ready for 'next' to me, too.

Thanks, all.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 02/12] t4150: am fails if index is dirty
  2015-07-05 15:38   ` Johannes Schindelin
@ 2015-07-07  6:35     ` Paul Tan
  0 siblings, 0 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-07  6:35 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git List, Stefan Beller, Junio C Hamano

On Sun, Jul 5, 2015 at 11:38 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> On 2015-07-02 20:16, Paul Tan wrote:
>
>> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
>> index 3f54bdf..0a19136 100755
>> --- a/t/t4150-am.sh
>> +++ b/t/t4150-am.sh
>> @@ -154,6 +154,17 @@ test_expect_success 'am applies patch correctly' '
>>       test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
>>  '
>>
>> +test_expect_success 'am fails if index is dirty' '
>> +     test_when_finished "rm -fr dirtyfile" &&
>
> Seeing as you `git add` that file further down, how about `git rm -f dirtfile` here?

But "git rm -f" would fail if the file is not in the index, no? (Which
could happen if the git-reset or git-checkout fails) Anyway, the
purpose of the "rm -f" is to remove the dirtyfile, and not to clean up
the index (we would use git reset --hard for that).

(But yeah, I see that Junio noticed correctly that it should not be a
rm -fr, but a rm -f instead.)

Thanks,
Paul

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 09/12] t4150: am with post-applypatch hook
  2015-07-05 15:58   ` Johannes Schindelin
@ 2015-07-07  6:47     ` Paul Tan
  2015-07-07  8:07       ` Johannes Schindelin
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Tan @ 2015-07-07  6:47 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git List, Stefan Beller, Junio C Hamano

On Sun, Jul 5, 2015 at 11:58 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> On 2015-07-02 20:16, Paul Tan wrote:
>
>> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
>> index dd6fe81..62b678c 100755
>> --- a/t/t4150-am.sh
>> +++ b/t/t4150-am.sh
>> @@ -275,6 +275,48 @@ test_expect_success 'am with failing pre-applypatch hook' '
>>       test_cmp_rev first HEAD
>>  '
>>
>> +test_expect_success 'am with post-applypatch hook' '
>> +     test_when_finished "rm -f .git/hooks/post-applypatch" &&
>> +     rm -fr .git/rebase-apply &&
>> +     git reset --hard &&
>> +     git checkout first &&
>> +     mkdir -p .git/hooks &&
>> +     cat >.git/hooks/post-applypatch <<-\EOF &&
>> +     #!/bin/sh
>> +     git rev-parse HEAD >head.actual
>> +     git diff second >diff.actual
>> +     exit 0
>> +     EOF
>> +     chmod +x .git/hooks/post-applypatch &&
>> +     git am patch1 &&
>> +     test_path_is_missing .git/rebase-apply &&
>> +     test_cmp_rev second HEAD &&
>> +     git rev-parse second >head.expected &&
>> +     test_cmp head.expected head.actual &&
>> +     git diff second >diff.expected &&
>> +     test_cmp diff.expected diff.actual
>> +'
>> +
>> +test_expect_success 'am with failing post-applypatch hook' '
>> +     test_when_finished "rm -f .git/hooks/post-applypatch" &&
>> +     rm -fr .git/rebase-apply &&
>> +     git reset --hard &&
>> +     git checkout first &&
>> +     mkdir -p .git/hooks &&
>> +     cat >.git/hooks/post-applypatch <<-\EOF &&
>> +     #!/bin/sh
>> +     git rev-parse HEAD >head.actual
>> +     exit 1
>> +     EOF
>> +     chmod +x .git/hooks/post-applypatch &&
>> +     git am patch1 &&
>> +     test_path_is_missing .git/rebase-apply &&
>> +     git diff --exit-code second &&
>> +     test_cmp_rev second HEAD &&
>> +     git rev-parse second >head.expected &&
>> +     test_cmp head.expected head.actual
>> +'
>
> These 2 tests as well as the previous patches look to me as if they could be refactored (the paradigm is the same: add a certain hook after resetting and then apply the patch, verify that the hook ran/failed)... do you think there is a chance for that?

I had a look, but I think that while it is true that the overall
sequence of each test is the same, the details differ enough that
there's no obvious way to refactor the tests sensibly. For example,
the contents of the hook scripts are not the same, as we need to check
that the hooks are run at the correct stage of git-am execution. And
as such, the verification tests are also different as well.

Junio did correctly point out though that we can shave off 2 lines if
the write_script helper is used (the shebang and the chmod).

Thanks,
Paul

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 09/12] t4150: am with post-applypatch hook
  2015-07-07  6:47     ` Paul Tan
@ 2015-07-07  8:07       ` Johannes Schindelin
  0 siblings, 0 replies; 26+ messages in thread
From: Johannes Schindelin @ 2015-07-07  8:07 UTC (permalink / raw)
  To: Paul Tan; +Cc: Git List, Stefan Beller, Junio C Hamano

Hi Paul,

On 2015-07-07 08:47, Paul Tan wrote:
> On Sun, Jul 5, 2015 at 11:58 PM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
>> On 2015-07-02 20:16, Paul Tan wrote:
>>
>>> diff --git a/t/t4150-am.sh b/t/t4150-am.sh
>>> index dd6fe81..62b678c 100755
>>> --- a/t/t4150-am.sh
>>> +++ b/t/t4150-am.sh
>>> @@ -275,6 +275,48 @@ test_expect_success 'am with failing pre-applypatch hook' '
>>>       test_cmp_rev first HEAD
>>>  '
>>>
>>> +test_expect_success 'am with post-applypatch hook' '
>>> +     test_when_finished "rm -f .git/hooks/post-applypatch" &&
>>> +     rm -fr .git/rebase-apply &&
>>> +     git reset --hard &&
>>> +     git checkout first &&
>>> +     mkdir -p .git/hooks &&
>>> +     cat >.git/hooks/post-applypatch <<-\EOF &&
>>> +     #!/bin/sh
>>> +     git rev-parse HEAD >head.actual
>>> +     git diff second >diff.actual
>>> +     exit 0
>>> +     EOF
>>> +     chmod +x .git/hooks/post-applypatch &&
>>> +     git am patch1 &&
>>> +     test_path_is_missing .git/rebase-apply &&
>>> +     test_cmp_rev second HEAD &&
>>> +     git rev-parse second >head.expected &&
>>> +     test_cmp head.expected head.actual &&
>>> +     git diff second >diff.expected &&
>>> +     test_cmp diff.expected diff.actual
>>> +'
>>> +
>>> +test_expect_success 'am with failing post-applypatch hook' '
>>> +     test_when_finished "rm -f .git/hooks/post-applypatch" &&
>>> +     rm -fr .git/rebase-apply &&
>>> +     git reset --hard &&
>>> +     git checkout first &&
>>> +     mkdir -p .git/hooks &&
>>> +     cat >.git/hooks/post-applypatch <<-\EOF &&
>>> +     #!/bin/sh
>>> +     git rev-parse HEAD >head.actual
>>> +     exit 1
>>> +     EOF
>>> +     chmod +x .git/hooks/post-applypatch &&
>>> +     git am patch1 &&
>>> +     test_path_is_missing .git/rebase-apply &&
>>> +     git diff --exit-code second &&
>>> +     test_cmp_rev second HEAD &&
>>> +     git rev-parse second >head.expected &&
>>> +     test_cmp head.expected head.actual
>>> +'
>>
>> These 2 tests as well as the previous patches look to me as if they could be refactored (the paradigm is the same: add a certain hook after resetting and then apply the patch, verify that the hook ran/failed)... do you think there is a chance for that?
> 
> I had a look, but I think that while it is true that the overall
> sequence of each test is the same, the details differ enough that
> there's no obvious way to refactor the tests sensibly. For example,
> the contents of the hook scripts are not the same, as we need to check
> that the hooks are run at the correct stage of git-am execution. And
> as such, the verification tests are also different as well.

Yeah, makes sense. Thanks for the clarification!

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 12/12] t3901: test git-am encoding conversion
  2015-07-02 18:16 ` [PATCH 12/12] t3901: test git-am encoding conversion Paul Tan
@ 2015-07-08 20:44   ` Johannes Sixt
  2015-07-14  9:43     ` Paul Tan
  0 siblings, 1 reply; 26+ messages in thread
From: Johannes Sixt @ 2015-07-08 20:44 UTC (permalink / raw)
  To: Paul Tan; +Cc: git, Johannes Schindelin, Stefan Beller, Junio C Hamano

Am 02.07.2015 um 20:16 schrieb Paul Tan:
> Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
> supported the --utf8 and --no-utf8 options, and if set, would pass the
> -u flag and the -k flag respectively.
>
> git mailinfo -u will re-code the commit log message and authorship info
> in the charset specified by i18n.commitencoding setting, while
> git mailinfo -n will disable the re-coding.
>
> Since d84029b (--utf8 is now default for 'git-am', 2007-01-08), --utf8
> is set by default in git-am.
>
> Add various encoding conversion tests to t3901 to test git-mailinfo's
> encoding conversion. In addition, add a test for --no-utf8 to check that
> no encoding conversion will occur if that option is set.
>
> Cc: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>   t/t3901-i18n-patch.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 62 insertions(+)
>
> diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
> index 75cf3ff..b49bdb7 100755
> --- a/t/t3901-i18n-patch.sh
> +++ b/t/t3901-i18n-patch.sh
> @@ -251,4 +251,66 @@ test_expect_success 'rebase --merge (L/U)' '
>   	check_encoding 2 8859
>   '
>
> +test_expect_success 'am (U/U)' '
> +	# Apply UTF-8 patches with UTF-8 commitencoding
> +	git config i18n.commitencoding UTF-8 &&
> +	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
> +
> +	git reset --hard master &&
> +	git am out-u1 out-u2 &&
> +
> +	check_encoding 2
> +'
> +
> +test_expect_success 'am (L/L)' '
> +	# Apply ISO-8859-1 patches with ISO-8859-1 commitencoding
> +	git config i18n.commitencoding ISO8859-1 &&
> +	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
> +
> +	git reset --hard master &&
> +	git am out-l1 out-l2 &&
> +
> +	check_encoding 2 8859
> +'

This test case must be protected by !MINGW, just like the last case 
below and other cases that are already in the file. See 32f4cb6cee for 
details.

> +
> +test_expect_success 'am (U/L)' '
> +	# Apply ISO-8859-1 patches with UTF-8 commitencoding
> +	git config i18n.commitencoding UTF-8 &&
> +	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
> +	git reset --hard master &&
> +
> +	# am specifies --utf8 by default.
> +	git am out-l1 out-l2 &&
> +
> +	check_encoding 2
> +'
> +
> +test_expect_success 'am --no-utf8 (U/L)' '
> +	# Apply ISO-8859-1 patches with UTF-8 commitencoding
> +	git config i18n.commitencoding UTF-8 &&
> +	. "$TEST_DIRECTORY"/t3901-utf8.txt &&
> +
> +	git reset --hard master &&
> +	git am --no-utf8 out-l1 out-l2 2>err &&
> +
> +	# commit-tree will warn that the commit message does not contain valid UTF-8
> +	# as mailinfo did not convert it
> +	grep "did not conform" err &&
> +
> +	check_encoding 2
> +'
> +
> +test_expect_success !MINGW 'am (L/U)' '
> +	# Apply UTF-8 patches with ISO-8859-1 commitencoding
> +	git config i18n.commitencoding ISO8859-1 &&
> +	. "$TEST_DIRECTORY"/t3901-8859-1.txt &&
> +
> +	git reset --hard master &&
> +	# mailinfo will re-code the commit message to the charset specified by
> +	# i18n.commitencoding
> +	git am out-u1 out-u2 &&
> +
> +	check_encoding 2 8859
> +'
> +
>   test_done
>

-- Hannes

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 12/12] t3901: test git-am encoding conversion
  2015-07-08 20:44   ` Johannes Sixt
@ 2015-07-14  9:43     ` Paul Tan
  0 siblings, 0 replies; 26+ messages in thread
From: Paul Tan @ 2015-07-14  9:43 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Git List, Johannes Schindelin, Stefan Beller, Junio C Hamano

On Thu, Jul 9, 2015 at 4:44 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 02.07.2015 um 20:16 schrieb Paul Tan:
>> diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
>> index 75cf3ff..b49bdb7 100755
>> --- a/t/t3901-i18n-patch.sh
>> +++ b/t/t3901-i18n-patch.sh
>> @@ -251,4 +251,66 @@ test_expect_success 'rebase --merge (L/U)' '
>>         check_encoding 2 8859
>>   '
>>
>> +test_expect_success 'am (U/U)' '
>> +       # Apply UTF-8 patches with UTF-8 commitencoding
>> +       git config i18n.commitencoding UTF-8 &&
>> +       . "$TEST_DIRECTORY"/t3901-utf8.txt &&
>> +
>> +       git reset --hard master &&
>> +       git am out-u1 out-u2 &&
>> +
>> +       check_encoding 2
>> +'
>> +
>> +test_expect_success 'am (L/L)' '
>> +       # Apply ISO-8859-1 patches with ISO-8859-1 commitencoding
>> +       git config i18n.commitencoding ISO8859-1 &&
>> +       . "$TEST_DIRECTORY"/t3901-8859-1.txt &&
>> +
>> +       git reset --hard master &&
>> +       git am out-l1 out-l2 &&
>> +
>> +       check_encoding 2 8859
>> +'
>
>
> This test case must be protected by !MINGW, just like the last case below
> and other cases that are already in the file. See 32f4cb6cee for details.

Ah, OK.

Thanks,
Paul

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2015-07-14  9:43 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-02 18:16 [PATCH 00/12] Improve git-am test coverage Paul Tan
2015-07-02 18:16 ` [PATCH 01/12] t4150: am.messageid really adds the message id Paul Tan
2015-07-02 18:41   ` Paolo Bonzini
2015-07-06 17:38     ` Junio C Hamano
2015-07-02 18:16 ` [PATCH 02/12] t4150: am fails if index is dirty Paul Tan
2015-07-05 15:38   ` Johannes Schindelin
2015-07-07  6:35     ` Paul Tan
2015-07-02 18:16 ` [PATCH 03/12] t4151: am --abort will keep dirty index intact Paul Tan
2015-07-02 18:16 ` [PATCH 04/12] t4150: am refuses patches when paused Paul Tan
2015-07-02 18:16 ` [PATCH 05/12] t4150: am --resolved fails if index has no changes Paul Tan
2015-07-02 18:16 ` [PATCH 06/12] t4150: am --resolved fails if index has unmerged entries Paul Tan
2015-07-02 18:16 ` [PATCH 07/12] t4150: am with applypatch-msg hook Paul Tan
2015-07-06 17:46   ` Junio C Hamano
2015-07-02 18:16 ` [PATCH 08/12] t4150: am with pre-applypatch hook Paul Tan
2015-07-02 18:16 ` [PATCH 09/12] t4150: am with post-applypatch hook Paul Tan
2015-07-05 15:58   ` Johannes Schindelin
2015-07-07  6:47     ` Paul Tan
2015-07-07  8:07       ` Johannes Schindelin
2015-07-02 18:16 ` [PATCH 10/12] t4150: tests for am --[no-]scissors Paul Tan
2015-07-02 18:16 ` [PATCH 11/12] t3418: non-interactive rebase --continue with rerere enabled Paul Tan
2015-07-02 18:16 ` [PATCH 12/12] t3901: test git-am encoding conversion Paul Tan
2015-07-08 20:44   ` Johannes Sixt
2015-07-14  9:43     ` Paul Tan
2015-07-03 16:24 ` [PATCH 00/12] Improve git-am test coverage Stefan Beller
2015-07-05 16:02   ` Johannes Schindelin
2015-07-06 20:42     ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).