Re: [PATCH v2 1/3] stash: add test to ensure reflog --rewrite --updatref behavior

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Feb 22 2022, John Cai via GitGitGadget wrote:

> From: John Cai <johncai86@gmail.com>
>
> There is missing test coverage to ensure that the resulting reflogs
> after a git stash drop has had its old oid rewritten if applicable, and
> if the refs/stash has been updated if applicable.

This test looks good, and if 3/3 is applied and either of the flags
you're passing is omitted they'll fail, so we know we have the missing
coverage here.

> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/t3903-stash.sh | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index b149e2af441..ec9cc5646d6 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -185,10 +185,33 @@ test_expect_success 'drop middle stash by index' '
>  	test 1 = $(git show HEAD:file)
>  '
>  
> +test_expect_success 'drop stash reflog updates refs/stash' '
> +	git reset --hard &&
> +	git rev-parse refs/stash >expect &&
> +	echo 9 >file &&
> +	git stash &&
> +	git stash drop stash@{0} &&
> +	git rev-parse refs/stash >actual &&
> +	test_cmp expect actual
> +'

This one will be portable to the reftable backend.

> +test_expect_success 'drop stash reflog updates refs/stash with rewrite' '

But as I noted in <220222.86fsob88h7.gmgdl@evledraar.gmail.com> (but it
was easy to miss) this test will need to depend on REFFILES. So just
changing this line to:

    test_expect_success REFFILES 'drop stash[...]'

> +	git reset --hard &&
> +	echo 9 >file &&
> +	git stash &&
> +	oid="$(git rev-parse stash@{0})" &&
> +	git stash drop stash@{1} &&
> +	cut -d" " -f1-2 .git/logs/refs/stash >actual &&
> +	cat >expect <<-EOF &&
> +	$(test_oid zero) $oid
> +	EOF
> +	test_cmp expect actual
> +'

Then:

>  test_expect_success 'stash pop' '
>  	git reset --hard &&
>  	git stash pop &&
> -	test 3 = $(cat file) &&
> +	test 9 = $(cat file) &&
>  	test 1 = $(git show :file) &&
>  	test 1 = $(git show HEAD:file) &&
>  	test 0 = $(git stash list | wc -l)

This test was already a bit broken in needing the preceding tests, but
it will break now if REFFILES isn't true, which you can reproduce
e.g. with:

    ./t3903-stash.sh --run=1-16,18-50 -vixd

Perhaps the least sucky solution to that is:

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ec9cc5646d6..1d11c9bda20 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -205,13 +205,19 @@ test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
 	cat >expect <<-EOF &&
 	$(test_oid zero) $oid
 	EOF
-	test_cmp expect actual
+	test_cmp expect actual &&
+	>dropped-stash
 '
 
 test_expect_success 'stash pop' '
 	git reset --hard &&
 	git stash pop &&
-	test 9 = $(cat file) &&
+	if test -e dropped-stash
+	then
+		test 9 = $(cat file)
+	else
+		test 3 = $(cat file)
+	fi &&
 	test 1 = $(git show :file) &&
 	test 1 = $(git show HEAD:file) &&
 	test 0 = $(git stash list | wc -l)