Re: Assertion in git log graphing [regression in v2.25]

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 7, 2020, 13:14 UTC
Message-ID: <CAPig+cTJKgXp1Pmp+muq-psHNrFhMBdZgfbMySozQ_dknpy-tw@mail.gmail.com>
In-Reply-To: <3fd8d52c-622e-29a6-442c-66797a3ae1f9@gmail.com>

On Tue, Jan 7, 2020 at 7:56 AM Derrick Stolee <stolee@gmail.com> wrote:

Show 11 quoted lines

> On 1/7/2020 7:50 AM, Eric Sunshine wrote:
> > On Tue, Jan 7, 2020 at 7:42 AM Derrick Stolee <stolee@gmail.com> wrote:
> >> Here is a a patch that reproduces the test failure. It hits the
> >> assert, so it definitely fails.
> >
> > What happens if someone builds the project with NDEBUG? Then the test
> > won't fail as expected. Perhaps this patch ought to also change those
> > asserts() to `if (...) BUG(...)` to ensure consistent behavior.
>
> A final version of the patch would probably fix the bug and not use
> test_expect_failure.

Indeed, I understood that when I made the suggestion, but you still want to be able to catch the problem if the code ever regresses and reintroduces the bug. And, to do so, you'd want to ensure consistent behavior which you get with BUG() but not with assert().