Subject: Re: [PATCH 6/6] builtin/reflog: introduce subcommand to list reflogs

From: Junio C Hamano <[email-masked]>
Date: Mon, 19 Feb 2024 16:32:43 -0800

Patrick Steinhardt <[email-masked]> writes:

> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> index d2f5f42e67..6d8d5a253d 100755
> --- a/t/t1410-reflog.sh
> +++ b/t/t1410-reflog.sh
> @@ -436,4 +436,73 @@ test_expect_success 'empty reflog' '
>  	test_must_be_empty err
>  '
>  
> +test_expect_success 'list reflogs' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		git reflog list >actual &&
> +		test_must_be_empty actual &&
> +
> +		test_commit A &&
> +		cat >expect <<-EOF &&
> +		HEAD
> +		refs/heads/main
> +		EOF
> +		git reflog list >actual &&
> +		test_cmp expect actual &&
> +
> +		git branch b &&
> +		cat >expect <<-EOF &&
> +		HEAD
> +		refs/heads/b
> +		refs/heads/main
> +		EOF
> +		git reflog list >actual &&
> +		test_cmp expect actual
> +	)
> +'

OK.  This is a quite boring baseline.

> +test_expect_success 'reflog list returns error with additional args' '
> +	cat >expect <<-EOF &&
> +	error: list does not accept arguments: ${SQ}bogus${SQ}
> +	EOF
> +	test_must_fail git reflog list bogus 2>err &&
> +	test_cmp expect err
> +'

Makes sense.

> +test_expect_success 'reflog for symref with unborn target can be listed' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit A &&
> +		git symbolic-ref HEAD refs/heads/unborn &&
> +		cat >expect <<-EOF &&
> +		HEAD
> +		refs/heads/main
> +		EOF
> +		git reflog list >actual &&
> +		test_cmp expect actual
> +	)
> +'

Should this be under REFFILES?  Ah, no, "git symbolic-ref" is valid
under reftable as well, so there is no need to.

Without [5/6], would it have failed to show the reflog for HEAD?

> +test_expect_success 'reflog with invalid object ID can be listed' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit A &&
> +		test-tool ref-store main update-ref msg refs/heads/missing \
> +			$(test_oid deadbeef) "$ZERO_OID" REF_SKIP_OID_VERIFICATION &&
> +		cat >expect <<-EOF &&
> +		HEAD
> +		refs/heads/main
> +		refs/heads/missing
> +		EOF
> +		git reflog list >actual &&
> +		test_cmp expect actual
> +	)
> +'

OK.

>  test_done

It would have been "interesting" to see an example of "there is a
reflog but the underlying ref for it is missing" case, but I think
that falls into a minor repository corruption category, so lack of
such a test is also fine.