Re: [PATCH 3/3] receive-pack: allow a maximum input size to be specified

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

 



On Tue, Aug 16, 2016 at 04:27:10PM +0200, Christian Couder wrote:

> >> +while read unpacklimit filesize filename
> >> +do
> >> [...]
> >> +done <<\EOF
> >> +1 1024 one-k-file
> >> +10 2048 two-k-file
> >> +EOF
> >
> > Is there any reason to use different filenames and sizes for the two
> > runs? They should behave identically, so it would make more sense to me
> > to subject them to identical inputs.
> 
> About the sizes, I thought that some people might want to try sizes
> closer to the limit and also that it is good anyway in general to add
> a bit of "randomness", or at least variability, in the tests.

In general, I'd prefer a systematic approach to introducing variables
into tests. If it's important that we test different sizes, then we
should do so, and not only test some combinations (and if it is not
important to test different sizes, then we should not waste CPU time and
the mental energy of people reading the tests).

IOW, when I see this, I wonder why the index-pack code path is not
tested against a 2k file. But there really isn't a good reason. So
either it does matter, and our tests do not have good coverage, or it
does not, and it is just making me wonder if the tests are buggy.

Worse, both files are created with the same seed via test-genrandom. So
I suspect the 2k file is a superset of the 1k file, and we may send it
is a thin delta (so it really is only testing a 1k input anyway!).

> I thought that it would be a bit less wasteful to use the same "dest"
> and also it would make the test more realistic as people often push in
> non empty repos.

I doubt it is expensive enough to matter in practice. Though note that
if you push the same commit to two new repositories, then you can
amortize the test-genrandom/add/commit step (i.e., push the exact same
packfile in both cases).

-Peff
--
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]