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

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?

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.