| Message ID | 20240814121122.4642-10-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:17PM +0530, Chandra Pratap wrote: > In the current testing setup, block operations are left unexercised > for obj blocks. Add a test that exercises these operations for obj > blocks. Same remarks here as for the preceding commit. > @@ -186,9 +186,88 @@ static void t_log_block_read_write(void) > reftable_record_release(&recs[i]); > } > > +static void t_obj_block_read_write(void) > +{ > + const int header_off = 21; > + 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 = { > + .last_key = STRBUF_INIT, > + }; > + struct reftable_record rec = { > + .type = BLOCK_TYPE_OBJ, > + }; > + size_t i = 0; > + int n; > + struct block_reader br = { 0 }; > + struct block_iter it = BLOCK_ITER_INIT; > + struct strbuf want = STRBUF_INIT; > + > + REFTABLE_CALLOC_ARRAY(block.data, block_size); > + block.len = block_size; > + block.source = malloc_block_source(); > + block_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size, > + header_off, hash_size(GIT_SHA1_FORMAT_ID)); > + > + for (i = 0; i < N; i++) { > + uint8_t *bytes = reftable_malloc(sizeof(uint8_t[5])); > + memcpy(bytes, (uint8_t[]){i, i+1, i+2, i+3, i+5}, sizeof(uint8_t[5])); From the top of my head I'm not sure whether we use inline-array declarations like this anywhere. I'd rather just make it a separate variable, which also allows us to get rid of the magic 5 via `ARRAY_SIZE()`. Patrick
On Thu, 15 Aug 2024 at 15:11, Patrick Steinhardt <ps@pks.im> wrote: > > On Wed, Aug 14, 2024 at 05:33:17PM +0530, Chandra Pratap wrote: > > In the current testing setup, block operations are left unexercised > > for obj blocks. Add a test that exercises these operations for obj > > blocks. > > Same remarks here as for the preceding commit. > > > @@ -186,9 +186,88 @@ static void t_log_block_read_write(void) > > reftable_record_release(&recs[i]); > > } > > > > +static void t_obj_block_read_write(void) > > +{ > > + const int header_off = 21; > > + 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 = { > > + .last_key = STRBUF_INIT, > > + }; > > + struct reftable_record rec = { > > + .type = BLOCK_TYPE_OBJ, > > + }; > > + size_t i = 0; > > + int n; > > + struct block_reader br = { 0 }; > > + struct block_iter it = BLOCK_ITER_INIT; > > + struct strbuf want = STRBUF_INIT; > > + > > + REFTABLE_CALLOC_ARRAY(block.data, block_size); > > + block.len = block_size; > > + block.source = malloc_block_source(); > > + block_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size, > > + header_off, hash_size(GIT_SHA1_FORMAT_ID)); > > + > > + for (i = 0; i < N; i++) { > > + uint8_t *bytes = reftable_malloc(sizeof(uint8_t[5])); > > + memcpy(bytes, (uint8_t[]){i, i+1, i+2, i+3, i+5}, sizeof(uint8_t[5])); > > From the top of my head I'm not sure whether we use inline-array > declarations like this anywhere. I'd rather just make it a separate > variable, which also allows us to get rid of the magic 5 via > `ARRAY_SIZE()`. We _do_ use inline array declarations like this, here's an example from t/unit-tests/t-prio-queue.c: TEST(TEST_INPUT(((int []){ STACK, 1, 2, 3, 4, 5, 6, REVERSE, DUMP }), ((int []){ 1, 2, 3, 4, 5, 6 })), "prio-queue works when LIFO stack is reversed"); I did implement bytes[] as a local variable array when I first worked on this patch but that turned out to be tricky due to variable scoping and pointer semantics, so I ultimately settled on this approach.
On Fri, Aug 16, 2024 at 12:41:34AM +0530, Chandra Pratap wrote: > On Thu, 15 Aug 2024 at 15:11, Patrick Steinhardt <ps@pks.im> wrote: > > > > On Wed, Aug 14, 2024 at 05:33:17PM +0530, Chandra Pratap wrote: > > > In the current testing setup, block operations are left unexercised > > > for obj blocks. Add a test that exercises these operations for obj > > > blocks. > > > > Same remarks here as for the preceding commit. > > > > > @@ -186,9 +186,88 @@ static void t_log_block_read_write(void) > > > reftable_record_release(&recs[i]); > > > } > > > > > > +static void t_obj_block_read_write(void) > > > +{ > > > + const int header_off = 21; > > > + 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 = { > > > + .last_key = STRBUF_INIT, > > > + }; > > > + struct reftable_record rec = { > > > + .type = BLOCK_TYPE_OBJ, > > > + }; > > > + size_t i = 0; > > > + int n; > > > + struct block_reader br = { 0 }; > > > + struct block_iter it = BLOCK_ITER_INIT; > > > + struct strbuf want = STRBUF_INIT; > > > + > > > + REFTABLE_CALLOC_ARRAY(block.data, block_size); > > > + block.len = block_size; > > > + block.source = malloc_block_source(); > > > + block_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size, > > > + header_off, hash_size(GIT_SHA1_FORMAT_ID)); > > > + > > > + for (i = 0; i < N; i++) { > > > + uint8_t *bytes = reftable_malloc(sizeof(uint8_t[5])); > > > + memcpy(bytes, (uint8_t[]){i, i+1, i+2, i+3, i+5}, sizeof(uint8_t[5])); > > > > From the top of my head I'm not sure whether we use inline-array > > declarations like this anywhere. I'd rather just make it a separate > > variable, which also allows us to get rid of the magic 5 via > > `ARRAY_SIZE()`. > > We _do_ use inline array declarations like this, here's an example from > t/unit-tests/t-prio-queue.c: > TEST(TEST_INPUT(((int []){ STACK, 1, 2, 3, 4, 5, 6, REVERSE, DUMP }), > ((int []){ 1, 2, 3, 4, 5, 6 })), "prio-queue works when LIFO > stack is reversed"); > > I did implement bytes[] as a local variable array when I first worked > on this patch but that turned out to be tricky due to variable scoping > and pointer semantics, so I ultimately settled on this approach. Oh, I didn't mean to say that you should _only_ use the local array. Rather something like this: uint8_t[] bytes = { i, i + 1, i + 2, i + 3, i + 5 }, *allocated; DUP_ARRAY(allocated, bytes, ARRAY_SIZE(bytes)); Patrick
diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c index 01ef10e7a6..34d37fe1a7 100644 --- a/t/unit-tests/t-reftable-block.c +++ b/t/unit-tests/t-reftable-block.c @@ -186,9 +186,88 @@ static void t_log_block_read_write(void) reftable_record_release(&recs[i]); } +static void t_obj_block_read_write(void) +{ + const int header_off = 21; + 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 = { + .last_key = STRBUF_INIT, + }; + struct reftable_record rec = { + .type = BLOCK_TYPE_OBJ, + }; + size_t i = 0; + int n; + struct block_reader br = { 0 }; + struct block_iter it = BLOCK_ITER_INIT; + struct strbuf want = STRBUF_INIT; + + REFTABLE_CALLOC_ARRAY(block.data, block_size); + block.len = block_size; + block.source = malloc_block_source(); + block_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size, + header_off, hash_size(GIT_SHA1_FORMAT_ID)); + + for (i = 0; i < N; i++) { + uint8_t *bytes = reftable_malloc(sizeof(uint8_t[5])); + memcpy(bytes, (uint8_t[]){i, i+1, i+2, i+3, i+5}, sizeof(uint8_t[5])); + + rec.u.obj.hash_prefix = bytes; + rec.u.obj.hash_prefix_len = 5; + + recs[i] = rec; + n = block_writer_add(&bw, &rec); + rec.u.obj.hash_prefix = NULL; + rec.u.obj.hash_prefix_len = 0; + check_int(n, ==, 0); + } + + n = block_writer_finish(&bw); + check_int(n, >, 0); + + block_writer_release(&bw); + + block_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ); + + block_iter_seek_start(&it, &br); + + for (i = 0; ; i++) { + int r = block_iter_next(&it, &rec); + check_int(r, >=, 0); + if (r > 0) + break; + check(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ)); + } + + for (i = 0; i < N; i++) { + block_iter_reset(&it); + reftable_record_key(&recs[i], &want); + + n = block_iter_seek_key(&it, &br, &want); + check_int(n, ==, 0); + + n = block_iter_next(&it, &rec); + check_int(n, ==, 0); + + check(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ)); + } + + block_reader_release(&br); + block_iter_close(&it); + reftable_record_release(&rec); + reftable_block_done(&br.block); + strbuf_release(&want); + for (i = 0; i < N; i++) + reftable_record_release(&recs[i]); +} + int cmd_main(int argc, const char *argv[]) { TEST(t_log_block_read_write(), "read-write operations on log blocks work"); + TEST(t_obj_block_read_write(), "read-write operations on obj blocks work"); TEST(t_ref_block_read_write(), "read-write operations on ref blocks work"); return test_done();
In the current testing setup, block operations are left unexercised for obj blocks. Add a test that exercises these operations for obj blocks. 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 | 79 +++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+)