Re: [PATCH v3 06/12] builtin/config.c: test misuse of format_config()

From: Jeff King <peff@peff.net>
Date: Jun 24, 2023, 01:31 UTC
Message-ID: <20230624013104.GE95358@coredump.intra.peff.net>
In-Reply-To: <20230623203219.3255267-1-jonathantanmy@google.com>

On Fri, Jun 23, 2023 at 01:32:19PM -0700, Jonathan Tan wrote:

Show 16 quoted lines

> "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> > +test_expect_success '--show-origin with --default' '
> > +	test_must_fail git config --show-origin --default foo some.key
> > +'
> 
> On my machine, this fails with
> 
>   BUG: config.c:4035: current_config_origin_type called outside config callback
>   /usr/local/google/home/jonathantanmy/git/t/test-lib-functions.sh: line 1067: 3255109 Aborted                 "$@" 2>&7
>   test_must_fail: died by signal 6: git config --show-origin --default foo some.key
> 
> (So it indeed fails, as expected, but test_must_fail seems to not like
> the exit code.)
> 
> Not sure if we need to be more precise with the exit code we're testing
> for.

That is test_must_fail operating as intended. We should _never_ hit a BUG() call, no matter what garbage we get from the user or the disk. The resolution is generally one of:

  - there really is a bug, so we should fix it

  - the invocation is garbage and expected to fail, but we should catch
    it sooner and give a nice error message, rather than getting as far
    as a BUG() call

But I didn't look at the patches to know which case this is.

-Peff