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
