| Message ID | 20240806142020.4615-5-chandrapratap3519@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | t: port reftable/stack_test.c to the unit testing framework | expand |
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. Patrick
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: static void t_reftable_ref_record_comparison(void) { struct reftable_record in[3] = { { .type = BLOCK_TYPE_REF, .u.ref.refname = (char *) "refs/heads/master", .u.ref.value_type = REFTABLE_REF_VAL1, }, { .type = BLOCK_TYPE_REF, .u.ref.refname = (char *) "refs/heads/master", .u.ref.value_type = REFTABLE_REF_DELETION, }, { .type = BLOCK_TYPE_REF, .u.ref.refname = (char *) "HEAD", .u.ref.value_type = REFTABLE_REF_SYMREF, .u.ref.value.symref = (char *) "refs/heads/master", }, }; check(!reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); check(!reftable_record_cmp(&in[0], &in[1])); check(!reftable_record_equal(&in[1], &in[2], GIT_SHA1_RAWSZ)); check_int(reftable_record_cmp(&in[1], &in[2]), >, 0); in[1].u.ref.value_type = in[0].u.ref.value_type; check(reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); check(!reftable_record_cmp(&in[0], &in[1])); }
On Wed, Aug 07, 2024 at 08:12:58PM +0530, Chandra Pratap wrote: > 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
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); printf("testing print functionality:\n"); @@ -262,8 +262,7 @@ static void t_reftable_stack_transaction_api(void) err = reftable_stack_read_ref(st, ref.refname, &dest); check(!err); - check_int(REFTABLE_REF_SYMREF, ==, dest.value_type); - check_str("master", dest.value.symref); + check(reftable_ref_record_equal(&ref, &dest, GIT_SHA1_RAWSZ)); reftable_ref_record_release(&dest); reftable_stack_destroy(st);
In the current stack tests, ref records are compared for equality by sometimes using the dedicated function for ref-record comparison, reftable_ref_record_equal(), and sometimes by explicity comparing contents of the ref records. Replace the latter instances of ref-record comparison with the former to maintain uniformity throughout the test file. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> --- t/unit-tests/t-reftable-stack.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)