diff mbox series

[v4,3/3] add -p tests: remove Perl prerequisite

Message ID 20240206225122.1095766-7-shyamthakkar001@gmail.com (mailing list archive)
State New, archived
Headers show
Series add-patch: '@' as a synonym for 'HEAD' | expand

Commit Message

Ghanshyam Thakkar Feb. 6, 2024, 10:50 p.m. UTC
The Perl version of the add -i/-p commands has been removed since
20b813d (add: remove "add.interactive.useBuiltin" & Perl "git
add--interactive", 2023-02-07)

Therefore, Perl prerequisite in t2071-restore-patch and
t7105-reset-patch is not necessary.

Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
---
 t/t2071-restore-patch.sh | 26 +++++++++++++-------------
 t/t7105-reset-patch.sh   | 22 +++++++++++-----------
 2 files changed, 24 insertions(+), 24 deletions(-)

Comments

Phillip Wood Feb. 7, 2024, 10:50 a.m. UTC | #1
Hi Ghanshyam

On 06/02/2024 22:50, Ghanshyam Thakkar wrote:
 > The Perl version of the add -i/-p commands has been removed since
 > 20b813d (add: remove "add.interactive.useBuiltin" & Perl "git
 > add--interactive", 2023-02-07)
 >
 > Therefore, Perl prerequisite in t2071-restore-patch and
 > t7105-reset-patch is not necessary.

Thanks for adding this patch. If you do re-roll I've just noticed that 
one of the tests in t7106-reset-unborn-branch.sh and another in 
t2024-checkout-dwim.sh still have PERL prerequisites as well. I don't 
think it is worth re-rolling just for that as we can clean them up 
separately if needed.

Best Wishes

Phillip

> Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> ---
>   t/t2071-restore-patch.sh | 26 +++++++++++++-------------
>   t/t7105-reset-patch.sh   | 22 +++++++++++-----------
>   2 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh
> index dbbefc188d..27e85be40a 100755
> --- a/t/t2071-restore-patch.sh
> +++ b/t/t2071-restore-patch.sh
> @@ -4,7 +4,7 @@ test_description='git restore --patch'
>   
>   . ./lib-patch-mode.sh
>   
> -test_expect_success PERL 'setup' '
> +test_expect_success 'setup' '
>   	mkdir dir &&
>   	echo parent >dir/foo &&
>   	echo dummy >bar &&
> @@ -16,28 +16,28 @@ test_expect_success PERL 'setup' '
>   	save_head
>   '
>   
> -test_expect_success PERL 'restore -p without pathspec is fine' '
> +test_expect_success 'restore -p without pathspec is fine' '
>   	echo q >cmd &&
>   	git restore -p <cmd
>   '
>   
>   # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar'
>   
> -test_expect_success PERL 'saying "n" does nothing' '
> +test_expect_success 'saying "n" does nothing' '
>   	set_and_save_state dir/foo work head &&
>   	test_write_lines n n | git restore -p &&
>   	verify_saved_state bar &&
>   	verify_saved_state dir/foo
>   '
>   
> -test_expect_success PERL 'git restore -p' '
> +test_expect_success 'git restore -p' '
>   	set_and_save_state dir/foo work head &&
>   	test_write_lines n y | git restore -p &&
>   	verify_saved_state bar &&
>   	verify_state dir/foo head head
>   '
>   
> -test_expect_success PERL 'git restore -p with staged changes' '
> +test_expect_success 'git restore -p with staged changes' '
>   	set_state dir/foo work index &&
>   	test_write_lines n y | git restore -p &&
>   	verify_saved_state bar &&
> @@ -56,7 +56,7 @@ do
>   	'
>   done
>   
> -test_expect_success PERL 'git restore -p --source=HEAD^' '
> +test_expect_success 'git restore -p --source=HEAD^' '
>   	set_state dir/foo work index &&
>   	# the third n is to get out in case it mistakenly does not apply
>   	test_write_lines n y n | git restore -p --source=HEAD^ &&
> @@ -64,7 +64,7 @@ test_expect_success PERL 'git restore -p --source=HEAD^' '
>   	verify_state dir/foo parent index
>   '
>   
> -test_expect_success PERL 'git restore -p --source=HEAD^...' '
> +test_expect_success 'git restore -p --source=HEAD^...' '
>   	set_state dir/foo work index &&
>   	# the third n is to get out in case it mistakenly does not apply
>   	test_write_lines n y n | git restore -p --source=HEAD^... &&
> @@ -72,7 +72,7 @@ test_expect_success PERL 'git restore -p --source=HEAD^...' '
>   	verify_state dir/foo parent index
>   '
>   
> -test_expect_success PERL 'git restore -p handles deletion' '
> +test_expect_success 'git restore -p handles deletion' '
>   	set_state dir/foo work index &&
>   	rm dir/foo &&
>   	test_write_lines n y | git restore -p &&
> @@ -85,21 +85,21 @@ test_expect_success PERL 'git restore -p handles deletion' '
>   # dir/foo.  There's always an extra 'n' to reject edits to dir/foo in
>   # the failure case (and thus get out of the loop).
>   
> -test_expect_success PERL 'path limiting works: dir' '
> +test_expect_success 'path limiting works: dir' '
>   	set_state dir/foo work head &&
>   	test_write_lines y n | git restore -p dir &&
>   	verify_saved_state bar &&
>   	verify_state dir/foo head head
>   '
>   
> -test_expect_success PERL 'path limiting works: -- dir' '
> +test_expect_success 'path limiting works: -- dir' '
>   	set_state dir/foo work head &&
>   	test_write_lines y n | git restore -p -- dir &&
>   	verify_saved_state bar &&
>   	verify_state dir/foo head head
>   '
>   
> -test_expect_success PERL 'path limiting works: HEAD^ -- dir' '
> +test_expect_success 'path limiting works: HEAD^ -- dir' '
>   	set_state dir/foo work head &&
>   	# the third n is to get out in case it mistakenly does not apply
>   	test_write_lines y n n | git restore -p --source=HEAD^ -- dir &&
> @@ -107,7 +107,7 @@ test_expect_success PERL 'path limiting works: HEAD^ -- dir' '
>   	verify_state dir/foo parent head
>   '
>   
> -test_expect_success PERL 'path limiting works: foo inside dir' '
> +test_expect_success 'path limiting works: foo inside dir' '
>   	set_state dir/foo work head &&
>   	# the third n is to get out in case it mistakenly does not apply
>   	test_write_lines y n n | (cd dir && git restore -p foo) &&
> @@ -115,7 +115,7 @@ test_expect_success PERL 'path limiting works: foo inside dir' '
>   	verify_state dir/foo head head
>   '
>   
> -test_expect_success PERL 'none of this moved HEAD' '
> +test_expect_success 'none of this moved HEAD' '
>   	verify_saved_head
>   '
>   
> diff --git a/t/t7105-reset-patch.sh b/t/t7105-reset-patch.sh
> index 7147148138..3691b94d1b 100755
> --- a/t/t7105-reset-patch.sh
> +++ b/t/t7105-reset-patch.sh
> @@ -5,7 +5,7 @@ test_description='git reset --patch'
>   TEST_PASSES_SANITIZE_LEAK=true
>   . ./lib-patch-mode.sh
>   
> -test_expect_success PERL 'setup' '
> +test_expect_success 'setup' '
>   	mkdir dir &&
>   	echo parent > dir/foo &&
>   	echo dummy > bar &&
> @@ -19,14 +19,14 @@ test_expect_success PERL 'setup' '
>   
>   # note: bar sorts before foo, so the first 'n' is always to skip 'bar'
>   
> -test_expect_success PERL 'saying "n" does nothing' '
> +test_expect_success 'saying "n" does nothing' '
>   	set_and_save_state dir/foo work work &&
>   	test_write_lines n n | git reset -p &&
>   	verify_saved_state dir/foo &&
>   	verify_saved_state bar
>   '
>   
> -test_expect_success PERL 'git reset -p' '
> +test_expect_success 'git reset -p' '
>   	test_write_lines n y | git reset -p >output &&
>   	verify_state dir/foo work head &&
>   	verify_saved_state bar &&
> @@ -43,28 +43,28 @@ do
>   	'
>   done
>   
> -test_expect_success PERL 'git reset -p HEAD^' '
> +test_expect_success 'git reset -p HEAD^' '
>   	test_write_lines n y | git reset -p HEAD^ >output &&
>   	verify_state dir/foo work parent &&
>   	verify_saved_state bar &&
>   	test_grep "Apply" output
>   '
>   
> -test_expect_success PERL 'git reset -p HEAD^^{tree}' '
> +test_expect_success 'git reset -p HEAD^^{tree}' '
>   	test_write_lines n y | git reset -p HEAD^^{tree} >output &&
>   	verify_state dir/foo work parent &&
>   	verify_saved_state bar &&
>   	test_grep "Apply" output
>   '
>   
> -test_expect_success PERL 'git reset -p HEAD^:dir/foo (blob fails)' '
> +test_expect_success 'git reset -p HEAD^:dir/foo (blob fails)' '
>   	set_and_save_state dir/foo work work &&
>   	test_must_fail git reset -p HEAD^:dir/foo &&
>   	verify_saved_state dir/foo &&
>   	verify_saved_state bar
>   '
>   
> -test_expect_success PERL 'git reset -p aaaaaaaa (unknown fails)' '
> +test_expect_success 'git reset -p aaaaaaaa (unknown fails)' '
>   	set_and_save_state dir/foo work work &&
>   	test_must_fail git reset -p aaaaaaaa &&
>   	verify_saved_state dir/foo &&
> @@ -76,27 +76,27 @@ test_expect_success PERL 'git reset -p aaaaaaaa (unknown fails)' '
>   # dir/foo.  There's always an extra 'n' to reject edits to dir/foo in
>   # the failure case (and thus get out of the loop).
>   
> -test_expect_success PERL 'git reset -p dir' '
> +test_expect_success 'git reset -p dir' '
>   	set_state dir/foo work work &&
>   	test_write_lines y n | git reset -p dir &&
>   	verify_state dir/foo work head &&
>   	verify_saved_state bar
>   '
>   
> -test_expect_success PERL 'git reset -p -- foo (inside dir)' '
> +test_expect_success 'git reset -p -- foo (inside dir)' '
>   	set_state dir/foo work work &&
>   	test_write_lines y n | (cd dir && git reset -p -- foo) &&
>   	verify_state dir/foo work head &&
>   	verify_saved_state bar
>   '
>   
> -test_expect_success PERL 'git reset -p HEAD^ -- dir' '
> +test_expect_success 'git reset -p HEAD^ -- dir' '
>   	test_write_lines y n | git reset -p HEAD^ -- dir &&
>   	verify_state dir/foo work parent &&
>   	verify_saved_state bar
>   '
>   
> -test_expect_success PERL 'none of this moved HEAD' '
> +test_expect_success 'none of this moved HEAD' '
>   	verify_saved_head
>   '
>
Phillip Wood Feb. 7, 2024, 1:51 p.m. UTC | #2
On 07/02/2024 10:50, Phillip Wood wrote:
> Hi Ghanshyam
> 
> On 06/02/2024 22:50, Ghanshyam Thakkar wrote:
>  > The Perl version of the add -i/-p commands has been removed since
>  > 20b813d (add: remove "add.interactive.useBuiltin" & Perl "git
>  > add--interactive", 2023-02-07)
>  >
>  > Therefore, Perl prerequisite in t2071-restore-patch and
>  > t7105-reset-patch is not necessary.
> 
> Thanks for adding this patch. If you do re-roll I've just noticed that 
> one of the tests in t7106-reset-unborn-branch.sh and another in 
> t2024-checkout-dwim.sh still have PERL prerequisites as well. I don't 
> think it is worth re-rolling just for that as we can clean them up 
> separately if needed.

I didn't cast my net wide enough when I was grepping earlier, 
t7514-commit-patch.sh and t3904-stash-patch.sh also have unnecessary 
PERL prerequisites

Best Wishes

Phillip
Junio C Hamano Feb. 7, 2024, 4:02 p.m. UTC | #3
Phillip Wood <phillip.wood123@gmail.com> writes:

> On 07/02/2024 10:50, Phillip Wood wrote:
>> Hi Ghanshyam
>> On 06/02/2024 22:50, Ghanshyam Thakkar wrote:
>>  > The Perl version of the add -i/-p commands has been removed since
>>  > 20b813d (add: remove "add.interactive.useBuiltin" & Perl "git
>>  > add--interactive", 2023-02-07)
>>  >
>>  > Therefore, Perl prerequisite in t2071-restore-patch and
>>  > t7105-reset-patch is not necessary.
>> Thanks for adding this patch. If you do re-roll I've just noticed
>> that one of the tests in t7106-reset-unborn-branch.sh and another in
>> t2024-checkout-dwim.sh still have PERL prerequisites as well. I
>> don't think it is worth re-rolling just for that as we can clean
>> them up separately if needed.
>
> I didn't cast my net wide enough when I was grepping earlier,
> t7514-commit-patch.sh and t3904-stash-patch.sh also have unnecessary
> PERL prerequisites

Thanks for helping usher this topic forward.  Very much appreciated.
Eric Sunshine Feb. 7, 2024, 4:58 p.m. UTC | #4
On Wed, Feb 7, 2024 at 8:51 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> On 07/02/2024 10:50, Phillip Wood wrote:
> > On 06/02/2024 22:50, Ghanshyam Thakkar wrote:
> >  > The Perl version of the add -i/-p commands has been removed since
> >  > 20b813d (add: remove "add.interactive.useBuiltin" & Perl "git
> >  > add--interactive", 2023-02-07)
> >  >
> >  > Therefore, Perl prerequisite in t2071-restore-patch and
> >  > t7105-reset-patch is not necessary.
> >
> > Thanks for adding this patch. If you do re-roll I've just noticed that
> > one of the tests in t7106-reset-unborn-branch.sh and another in
> > t2024-checkout-dwim.sh still have PERL prerequisites as well. I don't
> > think it is worth re-rolling just for that as we can clean them up
> > separately if needed.
>
> I didn't cast my net wide enough when I was grepping earlier,
> t7514-commit-patch.sh and t3904-stash-patch.sh also have unnecessary
> PERL prerequisites

Additionally, patch [2/3] drops a PERL prerequisite when it moves an
existing test into a loop, but the removal of the prerequisite is not
mentioned in the commit message. Presumably, the relocation-into-loop
and prerequisite-removal should have been done separately (in patches
[2/3] and [3/3], respectively), and that's how I'd suggest doing it.
diff mbox series

Patch

diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh
index dbbefc188d..27e85be40a 100755
--- a/t/t2071-restore-patch.sh
+++ b/t/t2071-restore-patch.sh
@@ -4,7 +4,7 @@  test_description='git restore --patch'
 
 . ./lib-patch-mode.sh
 
-test_expect_success PERL 'setup' '
+test_expect_success 'setup' '
 	mkdir dir &&
 	echo parent >dir/foo &&
 	echo dummy >bar &&
@@ -16,28 +16,28 @@  test_expect_success PERL 'setup' '
 	save_head
 '
 
-test_expect_success PERL 'restore -p without pathspec is fine' '
+test_expect_success 'restore -p without pathspec is fine' '
 	echo q >cmd &&
 	git restore -p <cmd
 '
 
 # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar'
 
-test_expect_success PERL 'saying "n" does nothing' '
+test_expect_success 'saying "n" does nothing' '
 	set_and_save_state dir/foo work head &&
 	test_write_lines n n | git restore -p &&
 	verify_saved_state bar &&
 	verify_saved_state dir/foo
 '
 
-test_expect_success PERL 'git restore -p' '
+test_expect_success 'git restore -p' '
 	set_and_save_state dir/foo work head &&
 	test_write_lines n y | git restore -p &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
 
-test_expect_success PERL 'git restore -p with staged changes' '
+test_expect_success 'git restore -p with staged changes' '
 	set_state dir/foo work index &&
 	test_write_lines n y | git restore -p &&
 	verify_saved_state bar &&
@@ -56,7 +56,7 @@  do
 	'
 done
 
-test_expect_success PERL 'git restore -p --source=HEAD^' '
+test_expect_success 'git restore -p --source=HEAD^' '
 	set_state dir/foo work index &&
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines n y n | git restore -p --source=HEAD^ &&
@@ -64,7 +64,7 @@  test_expect_success PERL 'git restore -p --source=HEAD^' '
 	verify_state dir/foo parent index
 '
 
-test_expect_success PERL 'git restore -p --source=HEAD^...' '
+test_expect_success 'git restore -p --source=HEAD^...' '
 	set_state dir/foo work index &&
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines n y n | git restore -p --source=HEAD^... &&
@@ -72,7 +72,7 @@  test_expect_success PERL 'git restore -p --source=HEAD^...' '
 	verify_state dir/foo parent index
 '
 
-test_expect_success PERL 'git restore -p handles deletion' '
+test_expect_success 'git restore -p handles deletion' '
 	set_state dir/foo work index &&
 	rm dir/foo &&
 	test_write_lines n y | git restore -p &&
@@ -85,21 +85,21 @@  test_expect_success PERL 'git restore -p handles deletion' '
 # dir/foo.  There's always an extra 'n' to reject edits to dir/foo in
 # the failure case (and thus get out of the loop).
 
-test_expect_success PERL 'path limiting works: dir' '
+test_expect_success 'path limiting works: dir' '
 	set_state dir/foo work head &&
 	test_write_lines y n | git restore -p dir &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
 
-test_expect_success PERL 'path limiting works: -- dir' '
+test_expect_success 'path limiting works: -- dir' '
 	set_state dir/foo work head &&
 	test_write_lines y n | git restore -p -- dir &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
 
-test_expect_success PERL 'path limiting works: HEAD^ -- dir' '
+test_expect_success 'path limiting works: HEAD^ -- dir' '
 	set_state dir/foo work head &&
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines y n n | git restore -p --source=HEAD^ -- dir &&
@@ -107,7 +107,7 @@  test_expect_success PERL 'path limiting works: HEAD^ -- dir' '
 	verify_state dir/foo parent head
 '
 
-test_expect_success PERL 'path limiting works: foo inside dir' '
+test_expect_success 'path limiting works: foo inside dir' '
 	set_state dir/foo work head &&
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines y n n | (cd dir && git restore -p foo) &&
@@ -115,7 +115,7 @@  test_expect_success PERL 'path limiting works: foo inside dir' '
 	verify_state dir/foo head head
 '
 
-test_expect_success PERL 'none of this moved HEAD' '
+test_expect_success 'none of this moved HEAD' '
 	verify_saved_head
 '
 
diff --git a/t/t7105-reset-patch.sh b/t/t7105-reset-patch.sh
index 7147148138..3691b94d1b 100755
--- a/t/t7105-reset-patch.sh
+++ b/t/t7105-reset-patch.sh
@@ -5,7 +5,7 @@  test_description='git reset --patch'
 TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-patch-mode.sh
 
-test_expect_success PERL 'setup' '
+test_expect_success 'setup' '
 	mkdir dir &&
 	echo parent > dir/foo &&
 	echo dummy > bar &&
@@ -19,14 +19,14 @@  test_expect_success PERL 'setup' '
 
 # note: bar sorts before foo, so the first 'n' is always to skip 'bar'
 
-test_expect_success PERL 'saying "n" does nothing' '
+test_expect_success 'saying "n" does nothing' '
 	set_and_save_state dir/foo work work &&
 	test_write_lines n n | git reset -p &&
 	verify_saved_state dir/foo &&
 	verify_saved_state bar
 '
 
-test_expect_success PERL 'git reset -p' '
+test_expect_success 'git reset -p' '
 	test_write_lines n y | git reset -p >output &&
 	verify_state dir/foo work head &&
 	verify_saved_state bar &&
@@ -43,28 +43,28 @@  do
 	'
 done
 
-test_expect_success PERL 'git reset -p HEAD^' '
+test_expect_success 'git reset -p HEAD^' '
 	test_write_lines n y | git reset -p HEAD^ >output &&
 	verify_state dir/foo work parent &&
 	verify_saved_state bar &&
 	test_grep "Apply" output
 '
 
-test_expect_success PERL 'git reset -p HEAD^^{tree}' '
+test_expect_success 'git reset -p HEAD^^{tree}' '
 	test_write_lines n y | git reset -p HEAD^^{tree} >output &&
 	verify_state dir/foo work parent &&
 	verify_saved_state bar &&
 	test_grep "Apply" output
 '
 
-test_expect_success PERL 'git reset -p HEAD^:dir/foo (blob fails)' '
+test_expect_success 'git reset -p HEAD^:dir/foo (blob fails)' '
 	set_and_save_state dir/foo work work &&
 	test_must_fail git reset -p HEAD^:dir/foo &&
 	verify_saved_state dir/foo &&
 	verify_saved_state bar
 '
 
-test_expect_success PERL 'git reset -p aaaaaaaa (unknown fails)' '
+test_expect_success 'git reset -p aaaaaaaa (unknown fails)' '
 	set_and_save_state dir/foo work work &&
 	test_must_fail git reset -p aaaaaaaa &&
 	verify_saved_state dir/foo &&
@@ -76,27 +76,27 @@  test_expect_success PERL 'git reset -p aaaaaaaa (unknown fails)' '
 # dir/foo.  There's always an extra 'n' to reject edits to dir/foo in
 # the failure case (and thus get out of the loop).
 
-test_expect_success PERL 'git reset -p dir' '
+test_expect_success 'git reset -p dir' '
 	set_state dir/foo work work &&
 	test_write_lines y n | git reset -p dir &&
 	verify_state dir/foo work head &&
 	verify_saved_state bar
 '
 
-test_expect_success PERL 'git reset -p -- foo (inside dir)' '
+test_expect_success 'git reset -p -- foo (inside dir)' '
 	set_state dir/foo work work &&
 	test_write_lines y n | (cd dir && git reset -p -- foo) &&
 	verify_state dir/foo work head &&
 	verify_saved_state bar
 '
 
-test_expect_success PERL 'git reset -p HEAD^ -- dir' '
+test_expect_success 'git reset -p HEAD^ -- dir' '
 	test_write_lines y n | git reset -p HEAD^ -- dir &&
 	verify_state dir/foo work parent &&
 	verify_saved_state bar
 '
 
-test_expect_success PERL 'none of this moved HEAD' '
+test_expect_success 'none of this moved HEAD' '
 	verify_saved_head
 '