Re: [PATCH v2 7/8] pack-bitmap: implement combined filter

From: Jeff King <peff@peff.net>
Date: Apr 9, 2021, 15:55 UTC
Message-ID: <YHB49723aQrh13yF@coredump.intra.peff.net>
In-Reply-To: <YHA359zqnw7MV8Bp@ncase>

On Fri, Apr 09, 2021 at 01:17:59PM +0200, Patrick Steinhardt wrote:

Show 16 quoted lines

> > Before this patch, I think your test:
> > 
> > > +test_expect_success 'combine filter' '
> > > +	git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
> > > +	git rev-list --use-bitmap-index \
> > > +		     --objects --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
> > > +	test_bitmap_traversal expect actual
> > > +'
> > 
> > would pass anyway, because we'd just skip using bitmaps. Is there a way
> > we can tell that the bitmap code actually kicked in? Maybe a perf test
> > would make it clear (those aren't always run, but hopefully we'd
> > eventually notice a regression there).
> 
> I think that's not actually true. Note that we're using
> `test_bitmap_traversal`:

Ah, right. I forgot about the hackery in test_bitmap_traversal() to let us tell the difference (even though I was the one who wrote it, I still consider it hackery ;) ).

So yes, this is a good test that we are allowing the combine filter.

-Peff