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@xxxxxxxxxxxxxxxxxxxxxxxxxx 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html