Re: Funny test flake failure: t7518-ident-corner-cases.sh, test 1

On Fri, Oct 16, 2020 at 12:29:26PM -0700, Elijah Newren wrote:

The issue comes from this test code:

       (
               sane_unset GIT_AUTHOR_EMAIL &&
               GIT_AUTHOR_NAME= &&
               test_must_fail git commit --allow-empty -m foo 2>err &&
               test_i18ngrep ! null err
       )

The last line expects 'null' to NOT be found in err, unfortunately,
the file err contained the line:

fatal: empty ident name (for
[off-list ref])
not allowed
218

Note that 'null' appears as a substring of the domain name, found
within 'gcliasfzo2nullsdbrimjtbyhg'.

Heh. That's an amusing find. Good detective work, and sorry I created
the problem.

+# confirm that we do not segfault _and_ that we do not say "(null)", as
+# glibc systems will quietly handle our NULL pointer

Should we tighten the test to check for "(null)" instead of "null", or
should we do something else?  Or just ignore it as it is somewhat
unlikely that anyone ever hits this flake again?

I think it's worth tightening the test. The "(null)" phrasing is pretty
common, and it's really the best we can do. The chance that somebody is
on a platform that neither segfaults not prints "(null)", _and_ that
they introduce a regression there seems pretty low. And in comparison,
you already wasted time tracking down a false negative. Let's make sure
that doesn't happen again.

I wish there was an environment variable or something we could set in
the test suite to convince glibc to actually segfault on NULL (because
it _will_ segfault on other platforms, and we're rather catch such
things sooner rather than later).

Another option, I guess, is that we probably _do_ run the tests on such
a platform (IIRC BSD will segfault, so macos probably does). So we could
just remove the grep entirely from this test and let any regression get
caught there.

Of the two, I think I have a slight preference for matching (null), but
I don't feel strongly.

-Peff