Re: [PATCH 4/6] t-reftable-stack: use reftable_ref_record_equal() to compare ref records

From: Patrick Steinhardt <ps@pks.im>
Date: Aug 8, 2024, 04:07 UTC
Message-ID: <ZrREaGKQmriAfnIV@tanuki>
In-Reply-To: <CA+J6zkTSBsrTZf2xyec-RaiqP3vJqvJmhVuW8GusbV-ia8CQ=A@mail.gmail.com>

On Wed, Aug 07, 2024 at 08:12:58PM +0530, Chandra Pratap wrote:

Show 23 quoted lines

> On Wed, 7 Aug 2024 at 14:11, Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Tue, Aug 06, 2024 at 07:43:40PM +0530, Chandra Pratap wrote:
> > > diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
> > > index 14909b127e..0c15e654e8 100644
> > > --- a/t/unit-tests/t-reftable-stack.c
> > > +++ b/t/unit-tests/t-reftable-stack.c
> > > @@ -145,7 +145,7 @@ static void t_reftable_stack_add_one(void)
> > >
> > >       err = reftable_stack_read_ref(st, ref.refname, &dest);
> > >       check(!err);
> > > -     check_str("master", dest.value.symref);
> > > +     check(reftable_ref_record_equal(&ref, &dest, GIT_SHA1_RAWSZ));
> > >       check_int(st->readers_len, >, 0);
> >
> > I think the change itself is sensible as long as we have tests that
> > verify that `reftable_ref_record_equal()` itself behaves as expected. I
> > don't think we have such tests anywhere though, uncovering a test gap.
> 
> We _do_ test reftable_record_equal (which is a wrapper for
> reftable_ref_record_equal in the case of ref records) in the
> recently ported t-reftable-record test. Here is the test exercising
> this function in unit-tests/t-reftable-record.c:

Ah, great, never mind then! I didn't see this test because we are using `reftable_ref_record_equal()` here, whereas the test uses `reftable_record_equal()`. But the latter uses the former for ref records, so that's fine.

Patrick