Re: [PATCH 2/8] reftable/stack: test compaction with already-locked tables

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +static void test_reftable_stack_auto_compaction_with_locked_tables(void)
> +{
> +	struct reftable_write_options opts = {
> +		.disable_auto_compact = 1,
> +	};
> +	struct reftable_stack *st = NULL;
> +	struct strbuf buf = STRBUF_INIT;
> +	char *dir = get_tmp_dir(__LINE__);
> +	int err;
> +
> +	err = reftable_new_stack(&st, dir, &opts);
> +	EXPECT_ERR(err);
> +
> +	for (size_t i = 0; i < 5; i++) {
> +		struct reftable_ref_record ref = {
> +			.update_index = reftable_stack_next_update_index(st),
> +			.value_type = REFTABLE_REF_VAL1,
> +			.value.val1 = { i },
> +		};

As val1 is an array of unsigned char, i cannot reasonably go beyond
255, but that is perfectly fine.  We are preparing 5 original tables
to compact, and that might grow to 17 tables over time, but 255 ought
to be more than enough.

> +
> +		strbuf_reset(&buf);
> +		strbuf_addf(&buf, "refs/heads/branch-%04" PRIuMAX, (uintmax_t) i);

Yet we are prepared to handle i that is beyond any usual integer ;-)

I am tempted to suggest using the bog-standard int for everything
for the sake of consistency within this loop, but it does not matter
all that much in a standalone test program ;-)

> +		ref.refname = buf.buf;
> +
> +		err = reftable_stack_add(st, &write_test_ref, &ref);
> +		EXPECT_ERR(err);
> +	}
> +	EXPECT(st->merged->stack_len == 5);
> +
> +	/*
> +	 * Given that all tables we have written should be roughly the same
> +	 * size, we expect that auto-compaction will want to compact all of the
> +	 * tables. Locking any of the tables will keep it from doing so.
> +	 */
> +	strbuf_reset(&buf);
> +	strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[2]->name);
> +	write_file_buf(buf.buf, "", 0);

OK.  [2] is just a random number pulled out of 0..5?

> +static void test_reftable_stack_compaction_with_locked_tables(void)
> +{
> +	struct reftable_write_options opts = {
> +		.disable_auto_compact = 1,
> +	};
> +	struct reftable_stack *st = NULL;
> +	struct strbuf buf = STRBUF_INIT;
> +	char *dir = get_tmp_dir(__LINE__);
> +	int err;
> +
> +	err = reftable_new_stack(&st, dir, &opts);
> +	EXPECT_ERR(err);
> +
> +	for (size_t i = 0; i < 3; i++) {
> +...
> +	}
> +	EXPECT(st->merged->stack_len == 3);

Hmph, this somehow looks familiar.  The only difference is how many
tables are compacted with which one locked, and whether it is
compact_all() or auto_compact() that triggers the compaction
behaviour, right?

I wonder if we want to factor out the commonality into a shared
function, or it is too much trouble only for two duplicates and we
can worry about it when we were about to add the third one?

Thanks.