git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johannes Sixt <j6t@kdbg.org>
Date
Nov 11, 2016, 21:15 UTC
Message-ID
<c4cdd21e-0cef-8255-75f2-90c84f271e02@kdbg.org>
In-Reply-To
<xmqqmvh5207k.fsf@gitster.mtv.corp.google.com>
Am 11.11.2016 um 22:09 schrieb Junio C Hamano:
Show 33 quoted lines
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> 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>
Perfect, thank you very much.
Show 31 quoted lines
> ---
>  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
>  '
>
>
Previous: Junio C Hamano
Message 9 of 9 in “t6026: ensure that long-running script really is”
  1. t6026: ensure that long-running script really isJohannes Schindelin, Nov 9, 2016
  2. Junio C HamanoNov 10, 2016
  3. Johannes SixtNov 11, 2016
  4. Jeff KingNov 11, 2016
  5. t6026: ensure that long-running script really isJohannes Sixt, Nov 11, 2016
  6. Junio C HamanoNov 11, 2016
  7. Johannes SixtNov 11, 2016
  8. Junio C HamanoNov 11, 2016
  9. Johannes SixtNov 11, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.