Re: [RFC 2/2] exclude-promisor-objects: declare when option is allowed

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 23, 2018, 05:08 UTC
Message-ID: <xmqqin1ts11h.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <931421945c040ba4518d91f7af9f386d0136bd2f.1540256910.git.matvore@google.com>

Matthew DeVore <matvore@google.com> writes:

Show 30 quoted lines

>  t/t4202-log.sh         | 4 ++++
>  t/t8002-blame.sh       | 4 ++++
>  7 files changed, 14 insertions(+), 1 deletion(-)
> ...
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 153a506151..819c24d10e 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -1703,4 +1703,8 @@ test_expect_success 'log --source paints symmetric ranges' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success '--exclude-promisor-objects does not BUG-crash' '
> +	test_must_fail git log --exclude-promisor-objects source-a
> +'
> +
>  test_done
> diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
> index 380e1c1054..eea048e52c 100755
> --- a/t/t8002-blame.sh
> +++ b/t/t8002-blame.sh
> @@ -118,4 +118,8 @@ test_expect_success '--no-abbrev works like --abbrev=40' '
>  	check_abbrev 40 --no-abbrev
>  '
>  
> +test_expect_success '--exclude-promisor-objects does not BUG-crash' '
> +	test_must_fail git blame --exclude-promisor-objects one
> +'
> +
>  test_done

OK. We used to be hitting BUG() which is an abort() in disguise, so must-fail would have caught it without the fix in this patch. Now we would see a more controlled failure.

    ... goes and makes sure that is the case ...

Not really. We were already doing a controlled failure via die(), so these two tests would not have caught the problem in the code before the fix in this patch.

But nevertheless this is a good change; I do not think it is worth grepping for "unrecognized option" to differentiate the two cases.

Thanks.