[v4,0/2] nd/merge-quit updates

Commit Message

Duy Nguyen May 18, 2019, 11:30 a.m. UTC

Another round because apparently the test case is not perfect.

Nguyễn Thái Ngọc Duy (2):
  merge: remove drop_save() in favor of remove_merge_branch_state()
  merge: add --quit

 Documentation/git-merge.txt |  4 ++++
 branch.c                    | 11 ++++++++---
 branch.h                    |  6 ++++++
 builtin/merge.c             | 30 ++++++++++++++++++------------
 t/t7600-merge.sh            | 26 ++++++++++++++++++++++++++
 5 files changed, 62 insertions(+), 15 deletions(-)

Interdiff dựa trên v3:

Comments

Johannes Schindelin May 20, 2019, 5:01 p.m. UTC | #1

Hi Duy,

On Sat, 18 May 2019, Nguyễn Thái Ngọc Duy wrote:

> Another round because apparently the test case is not perfect.

Test cases are never perfect. But at least the good ones are "actionable",
i.e. when they fail, you know there is a regression, it helps you figure
out easily why it broke, and it helps you fix the regression.

Many, many test cases are bad: they break for all kinds of reasons
*except* for a regression. They break because an error message was
changed. They break because of timing issues. They break because somebody
inserted another test case. They break because a missing prereq caused a
previous test case not to run, removing a side effect on which the
breaking test case relied.

Those bad test cases are very frustrating for people who actually look at
them. I am one of those people.

And if I did not know delightful test suites that are populated with
"good" test cases, I would not point out when a "bad" one is contributed.

So: it *is* possible to have a good test suite. Let's make ours better
than it is right now.

Thanks,
Johannes

Patch

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index d453710ef6..625a24a980 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -823,17 +823,29 @@  test_expect_success EXECKEEPSPID 'killed merge can be completed with --continue'
 '
 
 test_expect_success 'merge --quit' '
-	git reset --hard c2 &&
-	test_must_fail git -c rerere.enabled=true merge master &&
-	test_path_is_file .git/MERGE_HEAD &&
-	test_path_is_file .git/MERGE_MODE &&
-	test_path_is_file .git/MERGE_MSG &&
-	test_path_is_file .git/MERGE_RR &&
-	git merge --quit &&
-	test_path_is_missing .git/MERGE_HEAD &&
-	test_path_is_missing .git/MERGE_MODE &&
-	test_path_is_missing .git/MERGE_MSG &&
-	test_path_is_missing .git/MERGE_RR
+	git init merge-quit &&
+	(
+		cd merge-quit &&
+		test_commit base &&
+		echo one >>base.t &&
+		git commit -am one &&
+		git branch one &&
+		git checkout base &&
+		echo two >>base.t &&
+		git commit -am two &&
+		test_must_fail git -c rerere.enabled=true merge one &&
+		test_path_is_file .git/MERGE_HEAD &&
+		test_path_is_file .git/MERGE_MODE &&
+		test_path_is_file .git/MERGE_MSG &&
+		git rerere status >rerere.before &&
+		git merge --quit &&
+		test_path_is_missing .git/MERGE_HEAD &&
+		test_path_is_missing .git/MERGE_MODE &&
+		test_path_is_missing .git/MERGE_MSG &&
+		git rerere status >rerere.after &&
+		test_must_be_empty rerere.after &&
+		! test_cmp rerere.after rerere.before
+	)
 '
 
 test_done