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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]