Re: [PATCH 1/5] t7607: use test_commit and test_must_fail

[Jonathan Nieder <jrnieder@gmail.com>]

Clemens Buchacher wrote:

> [Subject: t7607: use test_commit and test_must_fail]
>
> Also make sure that aborted merges do not leave
> MERGE_HEAD except in the "will not overwrite
> removed file" test, where we cannot do so.

Motivation?

> See
> also the discussion in the following thread.
>
> http://mid.gmane.org/7vskopwxej.fsf@gitster.siamese.dyndns.org

That subthread is about teaching merge-recursive to write its results
to the index first without touching the worktree for robustness.  It
is not immediately obvious what that has to do with not leaving
MERGE_HEAD.  Could you summarize the relevant part for the commit
message so the reader does not have to track it down?

> --- a/t/t7607-merge-overwrite.sh
> +++ b/t/t7607-merge-overwrite.sh
> @@ -7,81 +7,73 @@ Do not overwrite changes.'
>  . ./test-lib.sh
>  
>  test_expect_success 'setup' '
> -	echo c0 > c0.c &&
> -	git add c0.c &&
> -	git commit -m c0 &&
> -	git tag c0 &&
> +	test_commit c0 &&

Behavior changes:

 . filename is c0.t instead of c0.c
 . uses test_tick for timestamp

Sounds good.

> -	echo c1 > c1.c &&
> -	git add c1.c &&
> -	git commit -m c1 &&
> -	git tag c1 &&
> +	test_commit c1 &&

Another almost-noop (good).

> -	git reset --hard c0 &&
> -	echo c2 > c2.c &&
> -	git add c2.c &&
> -	git commit -m c2 &&
> -	git tag c2 &&
> -	git reset --hard c1 &&
> -	echo "c1 a" > c1.c &&
> -	git add c1.c &&
> -	git commit -m "c1 a" &&
> -	git tag c1a &&
> +	test_commit "c1a" "c1.t" "c1 a" &&
> +	git reset --hard c0 &&
> +	test_commit c2 &&

The reader might not remember the c1.t filename.  Maybe
it would be good to be explicit in the 'test_commit c1' line:

	test_commit c1 c1.t

?  Or maybe a comment could help?

	# Commit rewriting the file from c1

>  	echo "VERY IMPORTANT CHANGES" > important
>  '
>  
>  test_expect_success 'will not overwrite untracked file' '
>  	git reset --hard c1 &&
> -	cat important > c2.c &&
> +	cp important c2.t &&
>  	test_must_fail git merge c2 &&
> -	test_cmp important c2.c
> +	! test -f .git/MERGE_HEAD &&
> +	test_cmp important c2.t
>  '

Nit: if backportability is not important, maybe

	test_path_is_missing .git/MERGE_HEAD &&

for better error messages when running tests with -v and
the file is present?

[...]
> -	cat important > c2.c &&
> -	git add c2.c &&
> -	rm c2.c &&
> +	cp important c2.t &&
> +	git add c2.t &&
> +	rm c2.t &&

These changes make for a lot of noise.  Why not specify the
filenames in the setup test to keep the .c extension?

The main change (checking that MERGE_HEAD is not present
for a merge that fails due to pre-merge checks) seems good.
Thanks.