[v2,09/11] t-reftable-block: add tests for log blocks

Commit Message

Chandra Pratap Aug. 16, 2024, 5:25 p.m. UTC

In the current testing setup, block operations are only exercised
for ref blocks. Add another test that exercises these operations
for log blocks as well.

Mentored-by: Patrick Steinhardt <[e-mail]>
Mentored-by: Christian Couder <[e-mail]>
Signed-off-by: Chandra Pratap <[e-mail]>
---
 t/unit-tests/t-reftable-block.c | 92 ++++++++++++++++++++++++++++++++-
 1 file changed, 90 insertions(+), 2 deletions(-)

Comments

Patrick Steinhardt Aug. 21, 2024, 7:28 a.m. UTC | #1

On Fri, Aug 16, 2024 at 10:55:32PM +0530, Chandra Pratap wrote:
> @@ -103,9 +103,97 @@ static void t_block_read_write(void)
>  		reftable_record_release(&recs[i]);
>  }
>  
> +static void t_log_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 = 2048;
> +	struct reftable_block block = { 0 };
> +	struct block_writer bw = {
> +		.last_key = STRBUF_INIT,
> +	};
> +	struct reftable_record rec = {
> +		.type = BLOCK_TYPE_LOG,
> +	};
> +	size_t i = 0;
> +	int ret;
> +	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_LOG, block.data, block_size,
> +			  header_off, hash_size(GIT_SHA1_FORMAT_ID));

Nit: instead of a `malloc_block_source()`, you may use
`block_source_from_strbuf()`. The former will go away with the patch
series at [1].

I'm also happy to rebase my patch series once yours lands and do this
myself. Guess yours will land faster anyway, and there are conflicts
regardless of whether you do or don't update the test here. The same
applies to the subsequent patches which use a `malloc_block_source()`.

So this isn't really worth a reroll by itself, and other than that this
patch looks good to me.

Patrick

[1]: <[e-mail]>

Junio C Hamano Aug. 21, 2024, 4:13 p.m. UTC | #2

Patrick Steinhardt <[e-mail]> writes:

>> +	REFTABLE_CALLOC_ARRAY(block.data, block_size);
>> +	block.len = block_size;
>> +	block.source = malloc_block_source();
>> +	block_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,
>> +			  header_off, hash_size(GIT_SHA1_FORMAT_ID));
>
> Nit: instead of a `malloc_block_source()`, you may use
> `block_source_from_strbuf()`. The former will go away with the patch
> series at [1].

I noticed that need for rewriting while merging the topic.  If the
patch uses the surviving alternative, that's one fewer thing I have
to worry about ;-).

Thanks.

Patch

diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c
index 6aa86a3edf..1256c7df6a 100644
--- a/t/unit-tests/t-reftable-block.c
+++ b/t/unit-tests/t-reftable-block.c
@@ -12,7 +12,7 @@  license that can be found in the LICENSE file or at
 #include "reftable/constants.h"
 #include "reftable/reftable-error.h"
 
-static void t_block_read_write(void)
+static void t_ref_block_read_write(void)
 {
 	const int header_off = 21; /* random */
 	struct reftable_record recs[30];
@@ -103,9 +103,97 @@  static void t_block_read_write(void)
 		reftable_record_release(&recs[i]);
 }
 
+static void t_log_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 = 2048;
+	struct reftable_block block = { 0 };
+	struct block_writer bw = {
+		.last_key = STRBUF_INIT,
+	};
+	struct reftable_record rec = {
+		.type = BLOCK_TYPE_LOG,
+	};
+	size_t i = 0;
+	int ret;
+	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_LOG, block.data, block_size,
+			  header_off, hash_size(GIT_SHA1_FORMAT_ID));
+
+	for (i = 0; i < N; i++) {
+		rec.u.log.refname = xstrfmt("branch%02"PRIuMAX , (uintmax_t)i);
+		rec.u.log.update_index = i;
+		rec.u.log.value_type = REFTABLE_LOG_UPDATE;
+
+		recs[i] = rec;
+		ret = block_writer_add(&bw, &rec);
+		rec.u.log.refname = NULL;
+		rec.u.log.value_type = REFTABLE_LOG_DELETION;
+		check_int(ret, ==, 0);
+	}
+
+	ret = block_writer_finish(&bw);
+	check_int(ret, >, 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++) {
+		ret = block_iter_next(&it, &rec);
+		check_int(ret, >=, 0);
+		if (ret > 0) {
+			check_int(i, ==, N);
+			break;
+		}
+		check(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));
+	}
+
+	for (i = 0; i < N; i++) {
+		block_iter_reset(&it);
+		strbuf_reset(&want);
+		strbuf_addstr(&want, recs[i].u.log.refname);
+
+		ret = block_iter_seek_key(&it, &br, &want);
+		check_int(ret, ==, 0);
+
+		ret = block_iter_next(&it, &rec);
+		check_int(ret, ==, 0);
+
+		check(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));
+
+		want.len--;
+		ret = block_iter_seek_key(&it, &br, &want);
+		check_int(ret, ==, 0);
+
+		ret = block_iter_next(&it, &rec);
+		check_int(ret, ==, 0);
+		check(reftable_record_equal(&recs[10 * (i / 10)], &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_block_read_write(), "read-write operations on blocks work");
+	TEST(t_log_block_read_write(), "read-write operations on log blocks work");
+	TEST(t_ref_block_read_write(), "read-write operations on ref blocks work");
 
 	return test_done();
 }