On Tue, Aug 16, 2016 at 6:21 PM, Jeff King <peff@xxxxxxxx> wrote: >> > >> > 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. I don't see things in the same way, but I will make the second file a 1k file too as I don't really care much about that. > 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!). Yeah, I will use a different seed for the second file. >> 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). Yeah, it probably doesn't matter anyway regarding efficiency, but I still prefer to not create a new repo as I would find it more confusing for the reader to use different repos. Thanks, Christian. -- 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