Subject: Re: [PATCH 10/12] t3903: move reffiles specific tests to t0600

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 19 Jan 2024 14:39:25 +0100

On Wed, Jan 17, 2024 at 07:52:33PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <[email-masked]>
> 
> Move this test into t0600 with other reffiles specific tests since it
> modifies reflog refs manually and thus is specific to the reffiles
> backend.
> 
> This change also consolidates setup_stash() into test-lib-functions.sh
> 
> Signed-off-by: John Cai <[email-masked]>
> ---
>  t/t0600-reffiles-backend.sh | 27 +++++++++++++++++++++++
>  t/t3903-stash.sh            | 43 -------------------------------------
>  t/test-lib-functions.sh     | 16 ++++++++++++++
>  3 files changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
> index 704b73fdc54..bee61b2d19d 100755
> --- a/t/t0600-reffiles-backend.sh
> +++ b/t/t0600-reffiles-backend.sh
> @@ -527,4 +527,31 @@ test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
>         test_must_fail git rev-parse --verify broken
>  '
>  
> +test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
> +	git init repo &&
> +	(
> +		cd repo &&
> +		setup_stash
> +	) &&
> +	echo 9 >repo/file &&
> +
> +	old_oid="$(git -C repo rev-parse stash@{0})" &&
> +	git -C repo stash &&
> +	new_oid="$(git -C repo rev-parse stash@{0})" &&
> +
> +	cat >expect <<-EOF &&
> +	$(test_oid zero) $old_oid
> +	$old_oid $new_oid
> +	EOF
> +	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> +	test_cmp expect actual &&
> +
> +	git -C repo stash drop stash@{1} &&
> +	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> +	cat >expect <<-EOF &&
> +	$(test_oid zero) $new_oid
> +	EOF
> +	test_cmp expect actual
> +'

I think that there is no need to make this backend-specific. What we're
testing here is that `git stash drop` is able to drop the latest reflog
entry. The calls to cut(1) are only used to verify that the contents of
the reflog entry look as expected while only verifying the old and new
object IDs.

So how about below patch to make it generic instead?

Patrick

-- >8 --

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 34faeac3f1..3319240515 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -200,7 +200,7 @@ test_expect_success 'drop stash reflog updates refs/stash' '
 	test_cmp expect actual
 '
 
-test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
+test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
 	git init repo &&
 	(
 		cd repo &&
@@ -213,16 +213,16 @@ test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite'
 	new_oid="$(git -C repo rev-parse stash@{0})" &&
 
 	cat >expect <<-EOF &&
-	$(test_oid zero) $old_oid
-	$old_oid $new_oid
+	$new_oid
+	$old_oid
 	EOF
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	git -C repo reflog show refs/stash --format=%H >actual &&
 	test_cmp expect actual &&
 
 	git -C repo stash drop stash@{1} &&
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	git -C repo reflog show refs/stash --format=%H >actual &&
 	cat >expect <<-EOF &&
-	$(test_oid zero) $new_oid
+	$new_oid
 	EOF
 	test_cmp expect actual
 '