From 734fde2d7167e4b20d2ff6062ade3846949b0741 Mon Sep 17 00:00:00 2001 From: Andreas Schwab Date: Tue, 8 Nov 2016 18:03:04 +0100 Subject: [PATCH 1/6] t6026-merge-attr: don't fail if sleep exits early Commit 5babb5bdb3 ("t6026-merge-attr: clean up background process at end of test case") added a kill command to clean up after the test, but this can fail if the sleep command exits before the cleanup is executed. Ignore the error from the kill command. Signed-off-by: Andreas Schwab Signed-off-by: Jeff King --- t/t6026-merge-attr.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh index 7a6e33e673..2672b15aa3 100755 --- a/t/t6026-merge-attr.sh +++ b/t/t6026-merge-attr.sh @@ -187,7 +187,7 @@ test_expect_success 'custom merge does not lock index' ' sleep 1 & echo $! >sleep.pid EOF - test_when_finished "kill \$(cat sleep.pid)" && + test_when_finished "kill \$(cat sleep.pid) || :" && test_write_lines >.gitattributes \ "* merge=ours" "text merge=sleep-one-second" && From c1e0dc59bddce765761a6f863c66ee0cd4b2ca09 Mon Sep 17 00:00:00 2001 From: Andreas Schwab Date: Thu, 10 Nov 2016 09:31:18 +0100 Subject: [PATCH 2/6] t6026-merge-attr: ensure that the merge driver was called Explicitly check for the existence of the pid file to test that the merge driver was actually called. Signed-off-by: Andreas Schwab Signed-off-by: Junio C Hamano --- t/t6026-merge-attr.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh index 2672b15aa3..03d13d00b5 100755 --- a/t/t6026-merge-attr.sh +++ b/t/t6026-merge-attr.sh @@ -193,7 +193,8 @@ test_expect_success 'custom merge does not lock index' ' "* merge=ours" "text merge=sleep-one-second" && test_config merge.ours.driver true && test_config merge.sleep-one-second.driver ./sleep-one-second.sh && - git merge master + git merge master && + test -f sleep.pid ' test_done From 3b03097d66ecc5798d1c289b84adc4a02b12cd7c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 10 Nov 2016 15:54:12 -0800 Subject: [PATCH 3/6] Revert "t6026-merge-attr: ensure that the merge driver was called" This reverts commit c1e0dc59bddce765761a6f863c66ee0cd4b2ca09. We are not interested in the stray process in the merge driver started; we want it to be still around. --- t/t6026-merge-attr.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh index 03d13d00b5..2672b15aa3 100755 --- a/t/t6026-merge-attr.sh +++ b/t/t6026-merge-attr.sh @@ -193,8 +193,7 @@ test_expect_success 'custom merge does not lock index' ' "* merge=ours" "text merge=sleep-one-second" && test_config merge.ours.driver true && test_config merge.sleep-one-second.driver ./sleep-one-second.sh && - git merge master && - test -f sleep.pid + git merge master ' test_done From b36b716cf611dea0054afef55241c88b99399571 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 10 Nov 2016 15:55:13 -0800 Subject: [PATCH 4/6] Revert "t6026-merge-attr: don't fail if sleep exits early" This reverts commit 734fde2d7167e4b20d2ff6062ade3846949b0741. The point of the test is that the stray process was still running when 'git merge' did its thing through its completion, so a failure to "kill" it means we didn't give a condition to the test to trigger a possible future breakage. Appending "|| :" to the "kill" is sweeping a test-bug under the rug. --- t/t6026-merge-attr.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh index 2672b15aa3..7a6e33e673 100755 --- a/t/t6026-merge-attr.sh +++ b/t/t6026-merge-attr.sh @@ -187,7 +187,7 @@ test_expect_success 'custom merge does not lock index' ' sleep 1 & echo $! >sleep.pid EOF - test_when_finished "kill \$(cat sleep.pid) || :" && + test_when_finished "kill \$(cat sleep.pid)" && test_write_lines >.gitattributes \ "* merge=ours" "text merge=sleep-one-second" && From a7d6bcc32936f3f0c4ba56b5f509648ffa85f63c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 9 Nov 2016 14:51:22 +0100 Subject: [PATCH 5/6] t6026: ensure that long-running script really is When making sure that background tasks are cleaned up in 5babb5b (t6026-merge-attr: clean up background process at end of test case, 2016-09-07), we considered to let the background task sleep longer, just to be certain that it will still be running when we want to kill it after the test. Sadly, the assumption appears not to hold true that the test case passes quickly enough to kill the background task within a second. Simply increase it to an hour. No system can be possibly slow enough to make above-mentioned assumption incorrect. Reported by Andreas Schwab. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t6026-merge-attr.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh index 7a6e33e673..348d78b205 100755 --- a/t/t6026-merge-attr.sh +++ b/t/t6026-merge-attr.sh @@ -183,16 +183,16 @@ test_expect_success 'up-to-date merge without common ancestor' ' test_expect_success 'custom merge does not lock index' ' git reset --hard anchor && - write_script sleep-one-second.sh <<-\EOF && - sleep 1 & + write_script sleep-an-hour.sh <<-\EOF && + sleep 3600 & echo $! >sleep.pid EOF test_when_finished "kill \$(cat sleep.pid)" && test_write_lines >.gitattributes \ - "* merge=ours" "text merge=sleep-one-second" && + "* merge=ours" "text merge=sleep-an-hour" && test_config merge.ours.driver true && - test_config merge.sleep-one-second.driver ./sleep-one-second.sh && + test_config merge.sleep-an-hour.driver ./sleep-an-hour.sh && git merge master ' From fdf4f6c79b4260e98729ebeb208036765595e9ac Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 11 Nov 2016 21:24:44 +0100 Subject: [PATCH 6/6] t6026: clarify the point of "kill $(cat sleep.pid)" We lengthened the time the leftover process sleeps in the previous commit to make sure it will be there while 'git merge' runs and finishes. It therefore needs to be killed before leaving the test. And it needs to be killed even when 'git merge' fails, so it has to be triggered via test_when_finished mechanism. Explain all that in a large comment, and move the use site of test_when_finished to immediately before 'git merge' invocation, where the process is spawned. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t6026-merge-attr.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh index 348d78b205..8f9b48a493 100755 --- a/t/t6026-merge-attr.sh +++ b/t/t6026-merge-attr.sh @@ -187,12 +187,20 @@ test_expect_success 'custom merge does not lock index' ' sleep 3600 & echo $! >sleep.pid EOF - test_when_finished "kill \$(cat sleep.pid)" && test_write_lines >.gitattributes \ "* merge=ours" "text merge=sleep-an-hour" && test_config merge.ours.driver true && test_config merge.sleep-an-hour.driver ./sleep-an-hour.sh && + + # We are testing that the custom merge driver does not block + # index.lock on Windows due to an inherited file handle. + # To ensure that the backgrounded process ran sufficiently + # long (and has been started in the first place), we do not + # ignore the result of the kill command. + # By packaging the command in test_when_finished, we get both + # the correctness check and the clean-up. + test_when_finished "kill \$(cat sleep.pid)" && git merge master '