From: Johannes Sixt Date: Fri, 11 Nov 2016 21:15:32 GMT Subject: Re: [PATCH v2] t6026: ensure that long-running script really is Message-ID: In-Reply-To: Am 11.11.2016 um 22:09 schrieb Junio C Hamano: > Johannes Sixt writes: > >> Am 11.11.2016 um 21:48 schrieb Junio C Hamano: >>> Johannes Sixt 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 > 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 > Signed-off-by: Junio C Hamano Perfect, thank you very much. > --- > 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 > ' > >