Re: [PATCH v3 7/9] t/lib-bitmap.sh: parameterize tests over reverse index source

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 25, 2022, 21:40 UTC
Message-ID: <YfBuUBOjWpvVOE/9@nand.local>
In-Reply-To: <20220124191503.2447008-1-jonathantanmy@google.com>

On Mon, Jan 24, 2022 at 11:15:03AM -0800, Jonathan Tan wrote:

Show 7 quoted lines

> Taylor Blau <me@ttaylorr.com> writes:
> > To prepare for reading the reverse index data out of the MIDX itself,
> > teach the `test_rev_exists` function to take an expected "source" for
> > the reverse index data.
>
> Thanks - up to here looks good. Thanks especially for patch 6, which was easy
> to verify using "--color-moved --color-moved-ws=allow-indentation-change".

Good, thanks for verifying.

Show 14 quoted lines

> > diff --git a/t/lib-bitmap.sh b/t/lib-bitmap.sh
> > index 48a8730a13..77b5f46a03 100644
> > --- a/t/lib-bitmap.sh
> > +++ b/t/lib-bitmap.sh
> > @@ -275,17 +275,23 @@ midx_pack_source () {
> >
> >  test_rev_exists () {
> >  	commit="$1"
> > +	kind="$2"
> >
> >  	test_expect_success 'reverse index exists' '
>
> To make it easier to understand test failures, we should probably
> include "kind" in the name of the test case.

Sure, that seems good to me.

> In a separate commit after this one, we should do it for the other
> blocks.

But I'm not as certain about this one. The tests are already separated into two different test scripts (t5326 for the in-MIDX .rev file, and t5327 for the external .rev file). So it should already be clear based on what test script you're running.

Thanks, Taylor