On Wed, Nov 18, 2020 at 07:32:25PM +0100, SZEDER Gábor wrote:
> On Tue, Nov 17, 2020 at 04:46:16PM -0500, Taylor Blau wrote:
> > - Harden the tests so that they pass under sha256-mode (thanks SZEDER,
> > and Peff).
>
> Fixing this is good, of course, but...
>
> > 16: 86d77fd085 ! 18: 5262daa330 pack-bitmap-write: build fewer intermediate bitmaps
> > @@ t/t5310-pack-bitmaps.sh: test_expect_success 'setup repo with moderate-sized his
> > '
> >
> > test_expect_success 'rev-list --test-bitmap verifies bitmaps' '
> > +@@ t/t5310-pack-bitmaps.sh: test_expect_success 'truncated bitmap fails gracefully (ewah)' '
> > + git rev-list --use-bitmap-index --count --all >expect &&
> > + bitmap=$(ls .git/objects/pack/*.bitmap) &&
> > + test_when_finished "rm -f $bitmap" &&
> > +- test_copy_bytes 256 <$bitmap >$bitmap.tmp &&
> > ++ test_copy_bytes 270 <$bitmap >$bitmap.tmp &&
> > + mv -f $bitmap.tmp $bitmap &&
> > + git rev-list --use-bitmap-index --count --all >actual 2>stderr &&
> > + test_cmp expect actual &&
>
> Please don't simply sneak in such a change without explaining it in
> the commit message.
Ah, I certainly didn't mean to go under the radar, so to speak ;-). From
my perspective, the final patch looks like it picked a magic number in
the same way was the original version of this patch did, so I didn't
think to add any more detail there.
I did try and highlight this a little bit in the patch just before the
one you're commenting on, though:
- Adding more tests in this area. Testing these truncation situations
are remarkably fragile to even subtle changes in the bitmap
generation. So, the resulting tests are likely to be quite brittle.
Thanks,
Taylor
