[1/2] t2016: require the PERL prereq only when necessary

Commit Message

Johannes Schindelin Nov. 30, 2021, 2:14 p.m. UTC

From: Johannes Schindelin <[e-mail]>

The scripted version of the interactive mode of `git add` still requires
Perl, but the built-in version does not. Let's only require the PERL
prereq if testing the scripted version.

This addresses a long-standing NEEDSWORK added in 35166b1fb54 (t2016:
add a NEEDSWORK about the PERL prerequisite, 2020-10-07).

Signed-off-by: Johannes Schindelin <[e-mail]>
---
 t/t2016-checkout-patch.sh | 42 ++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

Comments

Junio C Hamano Dec. 1, 2021, 10:25 p.m. UTC | #1

"Johannes Schindelin via GitGitGadget" <[e-mail]>
writes:

> From: Johannes Schindelin <[e-mail]>
>
> The scripted version of the interactive mode of `git add` still requires
> Perl, but the built-in version does not. Let's only require the PERL
> prereq if testing the scripted version.
>
> This addresses a long-standing NEEDSWORK added in 35166b1fb54 (t2016:
> add a NEEDSWORK about the PERL prerequisite, 2020-10-07).
>
> Signed-off-by: Johannes Schindelin <[e-mail]>
> ---
>  t/t2016-checkout-patch.sh | 42 ++++++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 20 deletions(-)

Good.  I suspect there may not be too many developers who lack PERL
prerequisite around here, so it is not like the built-in version was
not sufficiently tested in developers' environment without this
patch, but it is nice to see us move in this direction.  Of course,
when we remove the non-builtin version, we'd further be able to lose
these prerequisites.

> diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
> index abfd586c32b..71c5a15be00 100755
> --- a/t/t2016-checkout-patch.sh
> +++ b/t/t2016-checkout-patch.sh
> @@ -4,7 +4,13 @@ test_description='git checkout --patch'
>  
>  . ./lib-patch-mode.sh
>  
> -test_expect_success PERL 'setup' '
> +if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN false && ! test_have_prereq PERL
> +then
> +	skip_all='skipping interactive add tests, PERL not set'
> +	test_done
> +fi
> +
> +test_expect_success 'setup' '
>  	mkdir dir &&
>  	echo parent > dir/foo &&
>  	echo dummy > bar &&
> @@ -18,44 +24,40 @@ test_expect_success PERL 'setup' '
>  
>  # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar'
>  
> -# NEEDSWORK: Since the builtin add-p is used when $GIT_TEST_ADD_I_USE_BUILTIN
> -# is given, we should replace the PERL prerequisite with an ADD_I prerequisite
> -# which first checks if $GIT_TEST_ADD_I_USE_BUILTIN is defined before checking
> -# PERL.
> -test_expect_success PERL 'saying "n" does nothing' '
> +test_expect_success 'saying "n" does nothing' '

Johannes Schindelin Dec. 2, 2021, 2:57 p.m. UTC | #2

Hi Junio,

On Wed, 1 Dec 2021, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <[e-mail]>
> writes:
>
> > From: Johannes Schindelin <[e-mail]>
> >
> > The scripted version of the interactive mode of `git add` still requires
> > Perl, but the built-in version does not. Let's only require the PERL
> > prereq if testing the scripted version.
> >
> > This addresses a long-standing NEEDSWORK added in 35166b1fb54 (t2016:
> > add a NEEDSWORK about the PERL prerequisite, 2020-10-07).
> >
> > Signed-off-by: Johannes Schindelin <[e-mail]>
> > ---
> >  t/t2016-checkout-patch.sh | 42 ++++++++++++++++++++-------------------
> >  1 file changed, 22 insertions(+), 20 deletions(-)
>
> Good.  I suspect there may not be too many developers who lack PERL
> prerequisite around here, so it is not like the built-in version was
> not sufficiently tested in developers' environment without this
> patch, but it is nice to see us move in this direction.  Of course,
> when we remove the non-builtin version, we'd further be able to lose
> these prerequisites.

It is probably true that few developers miss the Perl prerequisite, in
particular given that you cannot even run Git's tests without having a
working Perl.

However, we have seen time and time again that developers do not really
run the test suite. For some, it takes way too long time (for some, 10
minutes is too long, but I remember that Jeff Hostetler reported a 5h
running time, and let's not forget about Cygwin and NonStop) and others
simply won't bother. So my focus lies more with the CI/PR builds. And
there, Windows specifically runs with NO_PERL. And you guessed it, to save
time.

Ciao,
Dscho

Patch

diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
index abfd586c32b..71c5a15be00 100755
--- a/t/t2016-checkout-patch.sh
+++ b/t/t2016-checkout-patch.sh
@@ -4,7 +4,13 @@  test_description='git checkout --patch'
 
 . ./lib-patch-mode.sh
 
-test_expect_success PERL 'setup' '
+if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN false && ! test_have_prereq PERL
+then
+	skip_all='skipping interactive add tests, PERL not set'
+	test_done
+fi
+
+test_expect_success 'setup' '
 	mkdir dir &&
 	echo parent > dir/foo &&
 	echo dummy > bar &&
@@ -18,44 +24,40 @@  test_expect_success PERL 'setup' '
 
 # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar'
 
-# NEEDSWORK: Since the builtin add-p is used when $GIT_TEST_ADD_I_USE_BUILTIN
-# is given, we should replace the PERL prerequisite with an ADD_I prerequisite
-# which first checks if $GIT_TEST_ADD_I_USE_BUILTIN is defined before checking
-# PERL.
-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 checkout -p &&
 	verify_saved_state bar &&
 	verify_saved_state dir/foo
 '
 
-test_expect_success PERL 'git checkout -p' '
+test_expect_success 'git checkout -p' '
 	test_write_lines n y | git checkout -p &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
 
-test_expect_success PERL 'git checkout -p with staged changes' '
+test_expect_success 'git checkout -p with staged changes' '
 	set_state dir/foo work index &&
 	test_write_lines n y | git checkout -p &&
 	verify_saved_state bar &&
 	verify_state dir/foo index index
 '
 
-test_expect_success PERL 'git checkout -p HEAD with NO staged changes: abort' '
+test_expect_success 'git checkout -p HEAD with NO staged changes: abort' '
 	set_and_save_state dir/foo work head &&
 	test_write_lines n y n | git checkout -p HEAD &&
 	verify_saved_state bar &&
 	verify_saved_state dir/foo
 '
 
-test_expect_success PERL 'git checkout -p HEAD with NO staged changes: apply' '
+test_expect_success 'git checkout -p HEAD with NO staged changes: apply' '
 	test_write_lines n y y | git checkout -p HEAD &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
 
-test_expect_success PERL 'git checkout -p HEAD with change already staged' '
+test_expect_success 'git checkout -p HEAD with change already staged' '
 	set_state dir/foo index index &&
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines n y n | git checkout -p HEAD &&
@@ -63,21 +65,21 @@  test_expect_success PERL 'git checkout -p HEAD with change already staged' '
 	verify_state dir/foo head head
 '
 
-test_expect_success PERL 'git checkout -p HEAD^...' '
+test_expect_success 'git checkout -p HEAD^...' '
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines n y n | git checkout -p HEAD^... &&
 	verify_saved_state bar &&
 	verify_state dir/foo parent parent
 '
 
-test_expect_success PERL 'git checkout -p HEAD^' '
+test_expect_success 'git checkout -p HEAD^' '
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines n y n | git checkout -p HEAD^ &&
 	verify_saved_state bar &&
 	verify_state dir/foo parent parent
 '
 
-test_expect_success PERL 'git checkout -p handles deletion' '
+test_expect_success 'git checkout -p handles deletion' '
 	set_state dir/foo work index &&
 	rm dir/foo &&
 	test_write_lines n y | git checkout -p &&
@@ -90,28 +92,28 @@  test_expect_success PERL 'git checkout -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 checkout -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 checkout -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' '
 	# the third n is to get out in case it mistakenly does not apply
 	test_write_lines y n n | git checkout -p HEAD^ -- dir &&
 	verify_saved_state bar &&
 	verify_state dir/foo parent parent
 '
 
-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 checkout -p foo) &&
@@ -119,11 +121,11 @@  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
 '
 
-test_expect_success PERL 'empty tree can be handled' '
+test_expect_success 'empty tree can be handled' '
 	test_when_finished "git reset --hard" &&
 	git checkout -p $(test_oid empty_tree) --
 '