diff mbox series

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

Message ID 56a9c8f20dd7c8f3e9401b2bd3929fb9c53c7d27.1705521155.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Group reffiles tests | expand

Commit Message

John Cai Jan. 17, 2024, 7:52 p.m. UTC 
From: John Cai <johncai86@gmail.com>

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 <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 27 +++++++++++++++++++++++
 t/t3903-stash.sh            | 43 -------------------------------------
 t/test-lib-functions.sh     | 16 ++++++++++++++
 3 files changed, 43 insertions(+), 43 deletions(-)

Comments

Patrick Steinhardt Jan. 19, 2024, 1:39 p.m. UTC | #1 
On Wed, Jan 17, 2024 at 07:52:33PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
> 
> 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 <johncai86@gmail.com>
> ---
>  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
 '
John Cai Jan. 19, 2024, 3:47 p.m. UTC | #2 
Hi Patrick,

On 19 Jan 2024, at 8:39, Patrick Steinhardt wrote:

> On Wed, Jan 17, 2024 at 07:52:33PM +0000, John Cai via GitGitGadget wrote:
>> From: John Cai <johncai86@gmail.com>
>>
>> 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 <johncai86@gmail.com>
>> ---
>>  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?

Nice catch. This sounds perfect to me.

>
> 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
>  '
diff mbox series

Patch

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
+'
+
 test_done
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 34faeac3f1c..0b0e7b19fdc 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -42,22 +42,6 @@  diff_cmp () {
 	rm -f "$1.compare" "$2.compare"
 }
 
-setup_stash() {
-	echo 1 >file &&
-	git add file &&
-	echo unrelated >other-file &&
-	git add other-file &&
-	test_tick &&
-	git commit -m initial &&
-	echo 2 >file &&
-	git add file &&
-	echo 3 >file &&
-	test_tick &&
-	git stash &&
-	git diff-files --quiet &&
-	git diff-index --cached --quiet HEAD
-}
-
 test_expect_success 'stash some dirty working directory' '
 	setup_stash
 '
@@ -200,33 +184,6 @@  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' '
-	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
-'
-
 test_expect_success 'stash pop' '
 	git reset --hard &&
 	git stash pop &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index b5eaf7fdc11..68a6c8402d0 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1958,3 +1958,19 @@  test_trailing_hash () {
 		test-tool hexdump |
 		sed "s/ //g"
 }
+
+# Stash some changes
+setup_stash() { echo 1 >file &&
+	git add file &&
+	echo unrelated >other-file &&
+	git add other-file &&
+	test_tick &&
+	git commit -m initial &&
+	echo 2 >file &&
+	git add file &&
+	echo 3 >file &&
+	test_tick &&
+	git stash &&
+	git diff-files --quiet &&
+	git diff-index --cached --quiet HEAD
+}