Re: [PATCH v2] t6026: ensure that long-running script really is

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 11, 2016, 21:09 UTC
Message-ID: <xmqqmvh5207k.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <ba2b3f08-87b1-c954-2020-e5a4bece7f83@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 11 quoted lines

> Am 11.11.2016 um 21:48 schrieb Junio C Hamano:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>> Good point. Here is an updated version.
>>
>> Unfortunately, I already took the version before this one and
>> started my integration cycle today.  I'll wiggle this in; it
>> essentially is about adding a big comment to explain what is going
>> on and then moving the when-finished down below it, right?
>
> Correct.

Thanks.  It has become like so:

-- >8 --
From: Johannes Sixt <j6t@kdbg.org>
Date: Fri, 11 Nov 2016 21:24:44 +0100
Subject: [PATCH] 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 <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 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
 '

-- 
2.11.0-rc0-149-gea84a1ce98