| Message ID | 20240814121122.4642-4-chandrapratap3519@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | t: port reftable/block_test.c to the unit testing framework | expand |
On Wed, Aug 14, 2024 at 05:33:11PM +0530, Chandra Pratap wrote: > In the current testing setup, operations like read and write for > reftable blocks as defined by reftable/block.{c, h} are verified by > comparing only the keys of input and output reftable records. This is > not ideal because there can exist inequal reftable records with the > same key. Use the dedicated function for record comparison, > reftable_record_equal() instead of key-based comparison. Nit: there should probably be a comma after the closing brace. > diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c > index 31d179a50a..baeb9c8b07 100644 > --- a/t/unit-tests/t-reftable-block.c > +++ b/t/unit-tests/t-reftable-block.c > @@ -15,8 +15,8 @@ license that can be found in the LICENSE file or at > static void t_block_read_write(void) > { > const int header_off = 21; /* random */ > - char *names[30]; > - const size_t N = ARRAY_SIZE(names); > + struct reftable_record recs[30]; > + const size_t N = ARRAY_SIZE(recs); > const size_t block_size = 1024; > struct reftable_block block = { 0 }; > struct block_writer bw = { > @@ -47,11 +47,11 @@ static void t_block_read_write(void) > char name[100]; > snprintf(name, sizeof(name), "branch%02"PRIuMAX , (uintmax_t)i); > > - rec.u.ref.refname = name; > + rec.u.ref.refname = xstrdup(name); > rec.u.ref.value_type = REFTABLE_REF_VAL1; > memset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ); > > - names[i] = xstrdup(name); > + recs[i] = rec; > n = block_writer_add(&bw, &rec); > rec.u.ref.refname = NULL; > rec.u.ref.value_type = REFTABLE_REF_DELETION; > @@ -72,7 +72,7 @@ static void t_block_read_write(void) > check_int(r, >=, 0); > if (r > 0) > break; > - check_str(names[j], rec.u.ref.refname); > + check(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ)); > j++; > } Okay. Because we're not only checking for the refname anymore, we now need to store the expected records as full records, which also requires us to allocate the refname. Makes sense. > @@ -90,7 +90,7 @@ static void t_block_read_write(void) > n = block_iter_next(&it, &rec); > check_int(n, ==, 0); > > - check_str(names[i], rec.u.ref.refname); > + check(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ)); > > want.len--; > n = block_iter_seek_key(&it, &br, &want); It would of course be great if we didn't only verify that SHA1 works as expected, but that we can also read and write SHA256 records. But that would be a new addition to the test suite that doesn't have to be part of this patch series. Patrick
diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c index 31d179a50a..baeb9c8b07 100644 --- a/t/unit-tests/t-reftable-block.c +++ b/t/unit-tests/t-reftable-block.c @@ -15,8 +15,8 @@ license that can be found in the LICENSE file or at static void t_block_read_write(void) { const int header_off = 21; /* random */ - char *names[30]; - const size_t N = ARRAY_SIZE(names); + struct reftable_record recs[30]; + const size_t N = ARRAY_SIZE(recs); const size_t block_size = 1024; struct reftable_block block = { 0 }; struct block_writer bw = { @@ -47,11 +47,11 @@ static void t_block_read_write(void) char name[100]; snprintf(name, sizeof(name), "branch%02"PRIuMAX , (uintmax_t)i); - rec.u.ref.refname = name; + rec.u.ref.refname = xstrdup(name); rec.u.ref.value_type = REFTABLE_REF_VAL1; memset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ); - names[i] = xstrdup(name); + recs[i] = rec; n = block_writer_add(&bw, &rec); rec.u.ref.refname = NULL; rec.u.ref.value_type = REFTABLE_REF_DELETION; @@ -72,7 +72,7 @@ static void t_block_read_write(void) check_int(r, >=, 0); if (r > 0) break; - check_str(names[j], rec.u.ref.refname); + check(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ)); j++; } @@ -82,7 +82,7 @@ static void t_block_read_write(void) for (i = 0; i < N; i++) { struct block_iter it = BLOCK_ITER_INIT; strbuf_reset(&want); - strbuf_addstr(&want, names[i]); + strbuf_addstr(&want, recs[i].u.ref.refname); n = block_iter_seek_key(&it, &br, &want); check_int(n, ==, 0); @@ -90,7 +90,7 @@ static void t_block_read_write(void) n = block_iter_next(&it, &rec); check_int(n, ==, 0); - check_str(names[i], rec.u.ref.refname); + check(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ)); want.len--; n = block_iter_seek_key(&it, &br, &want); @@ -98,7 +98,7 @@ static void t_block_read_write(void) n = block_iter_next(&it, &rec); check_int(n, ==, 0); - check_str(names[10 * (i / 10)], rec.u.ref.refname); + check(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ)); block_iter_close(&it); } @@ -108,7 +108,7 @@ static void t_block_read_write(void) reftable_block_done(&br.block); strbuf_release(&want); for (i = 0; i < N; i++) - reftable_free(names[i]); + reftable_record_release(&recs[i]); } int cmd_main(int argc, const char *argv[])
In the current testing setup, operations like read and write for reftable blocks as defined by reftable/block.{c, h} are verified by comparing only the keys of input and output reftable records. This is not ideal because there can exist inequal reftable records with the same key. Use the dedicated function for record comparison, reftable_record_equal() instead of key-based comparison. 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-block.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)