Subject: Re: [PATCH 04/11] t: convert tests to not write references via the filesystem

From: Junio C Hamano <[email-masked]>
Date: Wed, 18 Oct 2023 11:34:23 -0700

Patrick Steinhardt <[email-masked]> writes:

> -test_expect_success "fail to create $n" '
> -	test_when_finished "rm -f .git/$n_dir" &&
> -	touch .git/$n_dir &&
> -	test_must_fail git update-ref $n $A
> +test_expect_success "fail to create $n due to file/directory conflict" '
> +	test_when_finished "git update-ref -d refs/heads/gu" &&
> +	git update-ref refs/heads/gu $A &&
> +	test_must_fail git update-ref refs/heads/gu/fixes $A
>  '

OK, the original checks "if a random garbage file, which may not
necessarily be a ref, exists at $n_dir, we cannot create a ref at
$n_dir/fixes, due to D/F conflict" more directly, but as long as our
intention is to enforce the D/F restriction across different ref
backends [*], creating a ref at $n_dir and making sure $n_dir/fixes
cannot be created is an equivalent check that is better (because it
can be applied for other backends).

    Side note: there is no fundamental need to, though, and there
         are cases where being able to have the 'seen' branch and
         'seen/ps/ref-test-tools' branches at the same time is
         beneficial---packed-refs and ref-table backends would not
         have such an inherent limitation, but they can of course be
         castrated to match what files-backend can(not) do.

> @@ -222,7 +220,7 @@ test_expect_success 'delete symref without dereference when the referred ref is
>  
>  test_expect_success 'update-ref -d is not confused by self-reference' '
>  	git symbolic-ref refs/heads/self refs/heads/self &&
> -	test_when_finished "rm -f .git/refs/heads/self" &&
> +	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
>  	test_path_is_file .git/refs/heads/self &&

I trust that this will be corrected to use some wrapper around "git
symbolic-ref" (or an equivalent for it as a test-tool subcommand) in
some future patch, if not in this series?

>  	test_must_fail git update-ref -d refs/heads/self &&
>  	test_path_is_file .git/refs/heads/self

Likewise.

> @@ -230,7 +228,7 @@ test_expect_success 'update-ref -d is not confused by self-reference' '
>  
>  test_expect_success 'update-ref --no-deref -d can delete self-reference' '
>  	git symbolic-ref refs/heads/self refs/heads/self &&
> -	test_when_finished "rm -f .git/refs/heads/self" &&
> +	test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
>  	test_path_is_file .git/refs/heads/self &&
>  	git update-ref --no-deref -d refs/heads/self &&
>  	test_must_fail git show-ref --verify -q refs/heads/self

We already have the "ref is missing" test here.

I'll stop at this point for now; will hopefully continue in a
separate message later.  Thanks.