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

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

On Fri, Feb 25 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.
>
> Add two tests that verify both of these happen.
>
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/t3903-stash.sh | 65 ++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 54 insertions(+), 11 deletions(-)
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index b149e2af441..73785cf862f 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -41,20 +41,29 @@ diff_cmp () {
>  	rm -f "$1.compare" "$2.compare"
>  }
>  
> -test_expect_success 'stash some dirty working directory' '
> -	echo 1 >file &&
> -	git add file &&
> -	echo unrelated >other-file &&
> -	git add other-file &&
> +setup_stash() {
> +	repo_dir=$1
> +	if test -z $repo_dir; then
> +		repo_dir="."
> +	fi
> +
> +	echo 1 >$repo_dir/file &&
> +	git -C $repo_dir add file &&
> +	echo unrelated >$repo_dir/other-file &&
> +	git -C $repo_dir add other-file &&
>  	test_tick &&
> -	git commit -m initial &&
> -	echo 2 >file &&
> +	git -C $repo_dir commit -m initial &&
> +	echo 2 >$repo_dir/file &&
>  	git add file &&
> -	echo 3 >file &&
> +	echo 3 >$repo_dir/file &&
>  	test_tick &&
> -	git stash &&
> -	git diff-files --quiet &&
> -	git diff-index --cached --quiet HEAD
> +	git -C $repo_dir stash &&
> +	git -C $repo_dir diff-files --quiet &&
> +	git -C $repo_dir diff-index --cached --quiet HEAD
> +}
> +
> +test_expect_success 'stash some dirty working directory' '
> +	setup_stash
>  '
>  
>  cat >expect <<EOF
> @@ -185,6 +194,40 @@ 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
> +'
> +
> +test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
> +	git init repo &&
> +	setup_stash repo &&
> +	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
> +'
> +
>  test_expect_success 'stash pop' '
>  	git reset --hard &&
>  	git stash pop &&

I just looked at the post-image of this and didn't notice it was altered
existing code at first, so I was going to suggest just this:
	
	diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
	index 73785cf862f..422d1f7a306 100755
	--- a/t/t3903-stash.sh
	+++ b/t/t3903-stash.sh
	@@ -42,24 +42,14 @@ diff_cmp () {
	 }
	 
	 setup_stash() {
	-	repo_dir=$1
	-	if test -z $repo_dir; then
	-		repo_dir="."
	-	fi
	-
	-	echo 1 >$repo_dir/file &&
	-	git -C $repo_dir add file &&
	-	echo unrelated >$repo_dir/other-file &&
	-	git -C $repo_dir add other-file &&
	-	test_tick &&
	-	git -C $repo_dir commit -m initial &&
	-	echo 2 >$repo_dir/file &&
	+	test_commit initial file 1 &&
	+	test_commit second other-file unrelated &&
	+	echo 2 >file &&
	 	git add file &&
	-	echo 3 >$repo_dir/file &&
	-	test_tick &&
	-	git -C $repo_dir stash &&
	-	git -C $repo_dir diff-files --quiet &&
	-	git -C $repo_dir diff-index --cached --quiet HEAD
	+	echo 3 >file &&
	+	git stash &&
	+	git diff-files --quiet &&
	+	git diff-index --cached --quiet HEAD
	 }
	 
	 test_expect_success 'stash some dirty working directory' '
	@@ -206,7 +196,10 @@ test_expect_success 'drop stash reflog updates refs/stash' '
	 
	 test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
	 	git init repo &&
	-	setup_stash repo &&
	+	(
	+		cd repo &&
	+		setup_stash repo
	+	) &&
	 	echo 9 >repo/file &&
	 
	 	old_oid="$(git -C repo rev-parse stash@{0})" &&

But I think much better as a replacement is:
	
	diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
	index b149e2af441..9dd05a497bc 100755
	--- a/t/t3903-stash.sh
	+++ b/t/t3903-stash.sh
	@@ -41,7 +41,7 @@ diff_cmp () {
	 	rm -f "$1.compare" "$2.compare"
	 }
	 
	-test_expect_success 'stash some dirty working directory' '
	+setup_stash() {
	 	echo 1 >file &&
	 	git add file &&
	 	echo unrelated >other-file &&
	@@ -55,6 +55,10 @@ test_expect_success 'stash some dirty working directory' '
	 	git stash &&
	 	git diff-files --quiet &&
	 	git diff-index --cached --quiet HEAD
	+}
	+
	+test_expect_success 'stash some dirty working directory' '
	+	setup_stash
	 '
	 
	 cat >expect <<EOF
	@@ -185,6 +189,43 @@ 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
	+'
	+
	+test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
	+	git init repo &&
	+	(
	+		cd repo &&
	+		setup_stash repo
	+	) &&
	+	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
	+'
	+
	 test_expect_success 'stash pop' '
	 	git reset --hard &&
	 	git stash pop &&

I>e. the only reason you need to add that -C etc. is because you'd like
to set it up in a subdirectory. Easier to just add the tiny bity of
logic to make that a sub-shell instead.