git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 2, 2022, 18:52 UTC
Message-ID
<220302.86ee3k6v9a.gmgdl@evledraar.gmail.com>
In-Reply-To
<33299825fc47fb0e2bc54e1f125282e7825e1924.1645817452.git.gitgitgadget@gmail.com>
On Fri, Feb 25 2022, John Cai via GitGitGadget wrote:
Show 100 quoted lines
> 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.

Previous: John Cai via GitGitGadgetNext: John Cai via GitGitGadget
Message 45 of 63 in “libify reflog”
  1. 0/3 libify reflogJohn Cai via GitGitGadget, Feb 18, 2022
  2. 2/3 reflog: call reflog_delete from reflog.cJohn Cai via GitGitGadget, Feb 18, 2022
  3. Ævar Arnfjörð BjarmasonFeb 18, 2022
  4. Junio C HamanoFeb 18, 2022
  5. 3/3 stash: call reflog_delete from reflog.cJohn Cai via GitGitGadget, Feb 18, 2022
  6. Ævar Arnfjörð BjarmasonFeb 18, 2022
  7. Taylor BlauFeb 19, 2022
  8. John CaiFeb 22, 2022
  9. Ævar Arnfjörð BjarmasonFeb 22, 2022
  10. 1/3 reflog: libify delete reflog function and helpersJohn Cai via GitGitGadget, Feb 18, 2022
  11. Ævar Arnfjörð BjarmasonFeb 18, 2022
  12. Taylor BlauFeb 18, 2022
  13. Ævar Arnfjörð BjarmasonFeb 18, 2022
  14. Taylor BlauFeb 18, 2022
  15. John CaiFeb 21, 2022
  16. Taylor BlauFeb 21, 2022
  17. John CaiFeb 23, 2022
  18. Junio C HamanoFeb 18, 2022
  19. Ævar Arnfjörð BjarmasonFeb 19, 2022
  20. Taylor BlauFeb 19, 2022
  21. Junio C HamanoFeb 20, 2022
  22. Junio C HamanoFeb 18, 2022
  23. Ævar Arnfjörð BjarmasonFeb 18, 2022
  24. 0/3 libify reflogJohn Cai via GitGitGadget, Feb 22, 2022
  25. 1/3 stash: add test to ensure reflog --rewrite --updatref behaviorJohn Cai via GitGitGadget, Feb 22, 2022
  26. Ævar Arnfjörð BjarmasonFeb 23, 2022
  27. Junio C HamanoFeb 23, 2022
  28. John CaiFeb 23, 2022
  29. Ævar Arnfjörð BjarmasonFeb 23, 2022
  30. John CaiFeb 24, 2022
  31. Ævar Arnfjörð BjarmasonFeb 25, 2022
  32. Junio C HamanoFeb 25, 2022
  33. Junio C HamanoFeb 23, 2022
  34. John CaiFeb 23, 2022
  35. Junio C HamanoFeb 23, 2022
  36. John CaiFeb 24, 2022
  37. Ævar Arnfjörð BjarmasonFeb 23, 2022
  38. 2/3 reflog: libify delete reflog function and helpersJohn Cai via GitGitGadget, Feb 22, 2022
  39. Ævar Arnfjörð BjarmasonFeb 23, 2022
  40. John CaiFeb 23, 2022
  41. Junio C HamanoFeb 23, 2022
  42. 3/3 stash: call reflog_delete() in reflog.cJohn Cai via GitGitGadget, Feb 22, 2022
  43. 0/3 libify reflogJohn Cai via GitGitGadget, Feb 25, 2022
  44. 1/3 stash: add tests to ensure reflog --rewrite --updatref behaviorJohn Cai via GitGitGadget, Feb 25, 2022
  45. Ævar Arnfjörð BjarmasonMar 2, 2022
  46. 3/3 stash: call reflog_delete() in reflog.cJohn Cai via GitGitGadget, Feb 25, 2022
  47. 2/3 reflog: libify delete reflog function and helpersJohn Cai via GitGitGadget, Feb 25, 2022
  48. Taylor BlauFeb 25, 2022
  49. John CaiMar 2, 2022
  50. Ævar Arnfjörð BjarmasonMar 2, 2022
  51. 0/3 libify reflogJohn Cai via GitGitGadget, Mar 2, 2022
  52. 1/3 stash: add tests to ensure reflog --rewrite --updatref behaviorJohn Cai via GitGitGadget, Mar 2, 2022
  53. Junio C HamanoMar 2, 2022
  54. John CaiMar 3, 2022
  55. Phillip WoodMar 3, 2022
  56. Ævar Arnfjörð BjarmasonMar 3, 2022
  57. Phillip WoodMar 3, 2022
  58. John CaiMar 3, 2022
  59. Phillip WoodMar 8, 2022
  60. John CaiMar 8, 2022
  61. 3/3 stash: call reflog_delete() in reflog.cJohn Cai via GitGitGadget, Mar 2, 2022
  62. 2/3 reflog: libify delete reflog function and helpersJohn Cai via GitGitGadget, Mar 2, 2022
  63. Junio C HamanoMar 2, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.