Hi Duy,

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

> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 106148254d..625a24a980 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -822,4 +822,30 @@ test_expect_success EXECKEEPSPID 'killed merge can be completed with --continue'
>  	verify_parents $c0 $c1
>  '
>
> +test_expect_success 'merge --quit' '
> +	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
> +	)
> +'

Good test cases do not *need* to be excessively long. Something like this
should be conciser, and more importantly, less inviting to typos or other
bugs:

	test_commit quit-test &&
	test_commit quit-one quit-test.t one &&
	git reset --hard HEAD^ &&
	test_commit quit-two quit-test.t two &&

	test_must_fail git -c rerere.enabled=true merge one &&
	test_path_is_file .git/MERGE_HEAD &&
	git rerere status >rerere &&
	test -s rerere &&

	git merge --quit &&
	test_path_is_missing .git/MERGE_HEAD &&
	git rerere status >rerere &&
	test_must_be_empty rerere

Note that this does not do an exhaustive test for all the .git/MERGE_*
files: this test regression promises to verify that `git merge --quit`
works, it does not promise to verify that a failed `git merge` leaves all
of those files! Using just `MERGE_HEAD` as a tell-tale for that is plenty
sufficient.

Likewise, this test case does not verify that the output of `git rerere
status` is different before and after the `git merge --quit`. That is not
the point of the test, to make sure that they are different. The point is
to make sure that it is empty afterwards, but not empty beforehand.

Technically, your version of the test case verifies the same (if a file's
contents differ from an empty file's, then necessarily it is not empty,
but it requires some gymnastics to come to that conclusion). There is no
need for convoluted thinking in regression test cases. In fact, the easier
it is to understand the *intent* of a test case, the quicker the
investigation of any future bug, and consequently the faster the bug fix.

Also, the less you execute, the quicker the test case runs. That might not
sound like much, but we have over 20,000 test cases in our test suite.
That multiplication is really easy to compute in your head. If all of them
were written succinctly, I bet you would find it much less taxing on your
patience to actually run the full test suite from time to time ;-)

And this illustrates a very real cost of a slow test suite in addition to
the time: developers will run it less often, causing more regressions,
wasting even more time in the long run.

Ciao,
Johannes