Re: [PATCH v2 2/3] reftable/stack: allow locking of outdated stacks

["James Liu" <james@jamesliu.io>]

I just want to check my understanding of this test, since I think it's
the first time I've reviewed anything using this test harness:

On Wed Sep 18, 2024 at 2:32 PM AEST, Patrick Steinhardt wrote:
> diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
> index d62a9c1bed5..a37cc698d87 100644
> --- a/t/unit-tests/t-reftable-stack.c
> +++ b/t/unit-tests/t-reftable-stack.c
> @@ -271,7 +271,7 @@ static void t_reftable_stack_transaction_api(void)
>  
>  	reftable_addition_destroy(add);
>  
> -	err = reftable_stack_new_addition(&add, st);
> +	err = reftable_stack_new_addition(&add, st, 0);
>  	check(!err);
>  
>  	err = reftable_addition_add(add, write_test_ref, &ref);
> @@ -292,6 +292,68 @@ static void t_reftable_stack_transaction_api(void)
>  	clear_dir(dir);
>  }
>  
> +static void t_reftable_stack_transaction_with_reload(void)
> +{
> +	char *dir = get_tmp_dir(__LINE__);
> +	struct reftable_stack *st1 = NULL, *st2 = NULL;
> +	int err;
> +	struct reftable_addition *add = NULL;
> +	struct reftable_ref_record refs[2] = {
> +		{
> +			.refname = (char *) "refs/heads/a",
> +			.update_index = 1,
> +			.value_type = REFTABLE_REF_VAL1,
> +			.value.val1 = { '1' },
> +		},
> +		{
> +			.refname = (char *) "refs/heads/b",
> +			.update_index = 2,
> +			.value_type = REFTABLE_REF_VAL1,
> +			.value.val1 = { '1' },
> +		},
> +	};
> +	struct reftable_ref_record ref = { 0 };
> +

Create two reftable stacks that provide a view into the reftable tables
inside "dir".

> +	err = reftable_new_stack(&st1, dir, NULL);
> +	check(!err);
> +	err = reftable_new_stack(&st2, dir, NULL);
> +	check(!err);
> +

Successfully add refs[0] to the first stack using the transactional API.

> +	err = reftable_stack_new_addition(&add, st1, 0);
> +	check(!err);
> +	err = reftable_addition_add(add, write_test_ref, &refs[0]);
> +	check(!err);
> +	err = reftable_addition_commit(add);
> +	check(!err);
> +	reftable_addition_destroy(add);
> +
> +	/*
> +	 * The second stack is now outdated, which we should notice. We do not
> +	 * create the addition and lock the stack by default, but allow the
> +	 * reload to happen when REFTABLE_STACK_NEW_ADDITION_RELOAD is set.
> +	 */

We try to open a transaction via the second reftable stack, but the
this stack is outdated because we've written to "dir" when the previous
stack addition was committed.

> +	err = reftable_stack_new_addition(&add, st2, 0);
> +	check_int(err, ==, REFTABLE_OUTDATED_ERROR);

Try again, but supply the flag so it performs a reload internally. Write
refs[1] to "dir" by committing the transaction. 

> +	err = reftable_stack_new_addition(&add, st2, REFTABLE_STACK_NEW_ADDITION_RELOAD);
> +	check(!err);
> +	err = reftable_addition_add(add, write_test_ref, &refs[1]);
> +	check(!err);
> +	err = reftable_addition_commit(add);
> +	check(!err);
> +	reftable_addition_destroy(add);
> +

Asserts.

> +	for (size_t i = 0; i < ARRAY_SIZE(refs); i++) {
> +		err = reftable_stack_read_ref(st2, refs[i].refname, &ref);
> +		check(!err);
> +		check(reftable_ref_record_equal(&refs[i], &ref, GIT_SHA1_RAWSZ));
> +	}
> +
> +	reftable_ref_record_release(&ref);
> +	reftable_stack_destroy(st1);
> +	reftable_stack_destroy(st2);
> +	clear_dir(dir);
> +}
> +