Re: [PATCH 2/2] teach git-config to output large integers

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 21, 2013, 06:34 UTC
Message-ID: <20130821063458.GB2802@elie.Belkin>
In-Reply-To: <20130821050053.GA875@sigill.intra.peff.net>

Jeff King wrote:

> On Tue, Aug 20, 2013 at 09:38:41PM -0700, Jonathan Nieder wrote:

Show 13 quoted lines

>> That is what I was trying to get at in discussing the test.  It is not
>> "We would like --int to reject values higher than this, but some
>> platforms do not allow us to", but "Either rejecting this value, or
>> even better, computing the right size and printing it, is an
>> acceptable behavior, and this test checks for those."
>
> You are conflating the two patches, I think. The test we were discussing
> is for the _first_ patch, which fixes a bug in the range check. It is
> not meant to test git-config in particular, but to test that values
> higher than INT_MAX and lower than LONG_MAX are properly range-checked.
>
> Forget the second patch for a moment. I believe the first one is a bug
> fix that we would want even if we do not take the second patch at all.

Sure. I'm not conflating the patches. What I mean is that tests are supposed to test desirable behavior, whatever that is --- they are not about preventing all behavior changes but only about preventing regressions.

So talking about tests is a (perhaps overly roundabout) way to figure out the desirable behavior.

In particular, at first glance I would think computing 3 * 2^20 instead of erroring out would be a *good* behavior, not a regression. If that's right, it doesn't make sense to me to go to careful lengths either to test that git continues to error out on most platforms, or to introduce new options to ensure "git config --int" continues to error out.

That is what I am trying to understand. Everything about the first patch except for the test makes sense to me, but the test doesn't. As you noted, we know the test won't pass on some platforms. Why is it something we should *want* to pass?

Jonathan