Hi Hannes,

On Mon, 30 Apr 2018, Johannes Sixt wrote:

Am 30.04.2018 um 00:17 schrieb Johannes Schindelin:

quoted

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.

[...]

  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.

I thought so at first, too. However, what these test cases run is not Git
itself. Instead, they run a t/helper/ command *specifically* designed to
hit the BUG code path, probably to ensure that bugs in future code will
actually not be silently ignored, but do exit with an error.

If SIGABRT occurs as a result of BUG(), and we know that this happens for
certain cases, it means we have an unfixed bug.

Not in this case: The code in question is in
https://github.com/git/git/blob/v2.17.0/t/helper/test-ref-store.c#L190-L201
and it is called in a way that fails to have the required flags for the
operation. This would normally indicate a bug, but in this case, that is
exactly what the regression test tries to trigger: we *want* such a bug to
cause a failure.

I'll do my best to clarify that in the commit message.

Ciao,
Dscho