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

Re: [PATCH 2/6] t1406: prepare for the refs code to fail with BUG()

From
Johannes Sixt <j6t@kdbg.org>
Date
Apr 30, 2018, 19:32 UTC
Message-ID
<7087f0b9-1362-f8ca-315d-96d27b91b26b@kdbg.org>
In-Reply-To
<9bbfd73a8e03a888a5e9e8800d853ece518a8bf5.1525040253.git.johannes.schindelin@gmx.de>
Am 30.04.2018 um 00:17 schrieb Johannes Schindelin:
Show 59 quoted lines
> t1406 specifically verifies that certain code paths fail with a BUG: ...
> message.
> 
> In the upcoming commit, we will convert that message to be generated via
> BUG() instead of die("BUG: ..."), which implies SIGABRT instead of a
> regular exit code.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>   t/t1406-submodule-ref-store.sh | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh
> index e093782cc37..0ea3457cae3 100755
> --- a/t/t1406-submodule-ref-store.sh
> +++ b/t/t1406-submodule-ref-store.sh
> @@ -16,7 +16,7 @@ test_expect_success 'setup' '
>   '
>   
>   test_expect_success 'pack_refs() not allowed' '
> -	test_must_fail $RUN pack-refs 3
> +	test_must_fail ok=sigabrt $RUN pack-refs 3
>   '
>   
>   test_expect_success 'peel_ref(new-tag)' '
> @@ -27,15 +27,18 @@ test_expect_success 'peel_ref(new-tag)' '
>   '
>   
>   test_expect_success 'create_symref() not allowed' '
> -	test_must_fail $RUN create-symref FOO refs/heads/master nothing
> +	test_must_fail ok=sigabrt \
> +		$RUN create-symref FOO refs/heads/master nothing
>   '
>   
>   test_expect_success 'delete_refs() not allowed' '
> -	test_must_fail $RUN delete-refs 0 nothing FOO refs/tags/new-tag
> +	test_must_fail ok=sigabrt \
> +		$RUN delete-refs 0 nothing FOO refs/tags/new-tag
>   '
>   
>   test_expect_success 'rename_refs() not allowed' '
> -	test_must_fail $RUN rename-ref refs/heads/master refs/heads/new-master
> +	test_must_fail ok=sigabrt \
> +		$RUN rename-ref refs/heads/master refs/heads/new-master
>   '
>   
>   test_expect_success 'for_each_ref(refs/heads/)' '
> @@ -91,11 +94,11 @@ test_expect_success 'reflog_exists(HEAD)' '
>   '
>   
>   test_expect_success 'delete_reflog() not allowed' '
> -	test_must_fail $RUN delete-reflog HEAD
> +	test_must_fail ok=sigabrt $RUN delete-reflog HEAD
>   '
>   
>   test_expect_success 'create-reflog() not allowed' '
> -	test_must_fail $RUN create-reflog HEAD 1
> +	test_must_fail ok=sigabrt $RUN create-reflog HEAD 1
>   '

I can't quite follow the rationale for this change. A 'BUG' error exit must never be reached, otherwise it is a bug in the program by definition. It cannot be OK that SIGABRT is a valid result from Git.

If SIGABRT occurs as a result of BUG(), and we know that this happens for certain cases, it means we have an unfixed bug. Should then not run these cases under test_expect_failure instead of test_expect_success to identify them as known bugs?

Confused.
-- Hannes
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 4 of 15 in “Finish the conversion from die("BUG: ...") to BUG()”
  1. 0/6 Finish the conversion from die("BUG: ...") to BUG()Johannes Schindelin, Apr 29, 2018
  2. 1/6 test_must_fail: support ok=sigabrtJohannes Schindelin, Apr 29, 2018
  3. 2/6 t1406: prepare for the refs code to fail with BUG()Johannes Schindelin, Apr 29, 2018
  4. Johannes SixtApr 30, 2018
  5. Johannes SchindelinMay 1, 2018
  6. Duy NguyenMay 1, 2018
  7. Duy NguyenMay 1, 2018
  8. Junio C HamanoMay 2, 2018
  9. Johannes SchindelinMay 2, 2018
  10. Junio C HamanoMay 2, 2018
  11. 3/6 refs/*: report bugs using the BUG() macroJohannes Schindelin, Apr 29, 2018
  12. 4/6 run-command: use BUG() to report bugs, not die()Johannes Schindelin, Apr 29, 2018
  13. 5/6 Replace all die("BUG: ...") calls by BUG() onesJohannes Schindelin, Apr 29, 2018
  14. 6/6 Convert remaining die*(BUG) messagesJohannes Schindelin, Apr 29, 2018
  15. Eric SunshineApr 30, 2018

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.