On Fri, May 12, 2017 at 10:03:46PM -0400, Jeff King wrote:
> On Sat, May 13, 2017 at 12:31:31AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> > > + if (use_local_config && nongit)
> > > + die(_("--local only be used inside a git repository"));
> > > +
> >
> > It would be better to have a test for edge cases that are currently
> > only being discovered by users in the wild.
>
> I actually started on one earlier, but what would it check? We already
> die() in this case. Should we be grepping for the message? It seems more
> likely to me that we would change the message and cause a false positive
> than that there would be an actual regression.
>
> What I think might be more interesting is if die("BUG") could learn to
> exit with some error code that the test suite considered invalid. Like
> calling abort(), which would kill us with SIGABRT and cause
> test_must_fail to complain.
>
> On many systems that would also generate a coredump. Which is handy
> sometimes, but I wonder if it would be inconvenient for others. I guess
> that is no different than what a raised assert() would do.
>
> But if we were to do that, then the test could easily demonstrate that
> we expect a clean die().
So here's a series which does that (and fixes the gramm-o that Jonathan
pointed out). The SIGABRT/coredump thing may seem like overkill, but
it's actually something I've found useful before (while developing this
very same setup_git_env assertion, in fact).
[1/3]: usage.c: add BUG() function
[2/3]: setup_git_env: convert die("BUG") to BUG()
[3/3]: config: complain about --local outside of a git repo
builtin/config.c | 3 +++
environment.c | 2 +-
git-compat-util.h | 9 +++++++++
t/t1300-repo-config.sh | 6 ++++++
usage.c | 32 ++++++++++++++++++++++++++++++++
5 files changed, 51 insertions(+), 1 deletion(-)
