On Thu, Jul 25, 2013 at 5:12 AM, Johannes Sixt <j.s...@viscovery.net> wrote:
> Am 7/25/2013 10:03, schrieb Eric Sunshine:
>> 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.
>> * Should we drop these new t4211 tests which guard against real potential
>> * 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.
You understand correctly. The added t4211 tests check range-set and
range-set is an implementation detail of git-log's -L and is entirely
private (static to the implementation file), so there's no API to test
via a test-foo program. It is sufficiently generic that its API could
(some day) be published, thus allowing a test-foo program, however,
doing so would involve writing documentation and covering its entire
API with tests: a large enough task in itself, and quite orthogonal to
fixing the log/blame -L loophole.
line-log is partially public, however, the code in which the bug was
discovered is private (static) and likely always will be since it is
not generic. Moreover, once the -L loophole is closed, there will be
no way to trigger the case under consideration via its public API, so
again there is no opportunity for a test-foo program.
Thus, the question remains: What to do with these two tests once the
-L loophole is closed? Remove them?
> 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.
Indeed, nicely stated.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html