Re: [PATCH 0/5] range-set and line-log bug fixes

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Jul 25, 2013, 09:12 UTC
Message-ID: <51F0EBF5.80105@viscovery.net>
In-Reply-To: <CAPig+cRWaKv6UN5pZLwonXBpHnFE7+4ELXTnE+2nLr8h83bX6Q@mail.gmail.com>

Am 7/25/2013 10:03, schrieb Eric Sunshine:

Show 5 quoted lines

> The tests in this series identify real bugs in dealing with empty
> ranges, which the subsequent patches fix. The test are possible
> because one can specify an empty range via blame/log -L, however, I
> now realize that the ability for -L to create empty ranges was never
> intended or part of the design, but is in fact itself a bug.

...

Show 7 quoted lines

> * Should we drop these new t4211 tests which guard against real potential bugs?
> 
> * Should we add custom C code to the test suite to make the
> empty-range testing possible?
> 
> * Should we introduce another (undocumented) loophole just for the
> sake of the tests?

IIUC, the tests you added are protecting the *implementation* of range-set functions. For tests of the implementation, we usually write test-foo programs that call the functions directly.

Tests invoking git should test the observable behavior. Therefore, if calling a git utility with "-Lfoo,+0" should be an error, then the test suite should mark such a call with test_must_fail. I guess this rules out the loophole approach.

-- Hannes