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

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

quoted

quoted

+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