[PATCH] tests: introduce test_unset_prereq, for debugging

[PATCH] tests: introduce test_unset_prereq, for debugging

[Johannes Schindelin]

While working on the --convert-graft-file test, I missed that I was
relying on the GPG prereq, by using output of test cases that were only
run under that prereq.

For debugging, it was really convenient to force that prereq to be
unmet, but there was no easy way to do that. So I came up with a way,
and this patch reflects the cleaned-up version of that way.

For convenience, the following two methods are now supported ways to
pretend that a prereq is not met:

	test_set_prereq !GPG

and

	test_unset_prereq GPG

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/test-unset-prereq-v1
Fetch-It-Via: git fetch https://github.com/dscho/git test-unset-prereq-v1
 t/test-lib-functions.sh | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 7d620bf2a9a..76cd6630f29 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -278,8 +278,20 @@ write_script () {
 # The single parameter is the prerequisite tag (a simple word, in all
 # capital letters by convention).
 
+test_unset_prereq () {
+	! test_have_prereq "$1" ||
+	satisfied_prereq="${satisfied_prereq% $1 *} ${satisfied_prereq#* $1 }"
+}
+
 test_set_prereq () {
-	satisfied_prereq="$satisfied_prereq$1 "
+	case "$1" in
+	!*)
+		test_unset_prereq "${1#!}"
+		;;
+	*)
+		satisfied_prereq="$satisfied_prereq$1 "
+		;;
+	esac
 }
 satisfied_prereq=" "
 lazily_testable_prereq= lazily_tested_prereq=

base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d
-- 
2.17.0.windows.1.36.gdf4ca5fb72a

Re: [PATCH] tests: introduce test_unset_prereq, for debugging

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> While working on the --convert-graft-file test, I missed that I was
> relying on the GPG prereq, by using output of test cases that were only
> run under that prereq.
>
> For debugging, it was really convenient to force that prereq to be
> unmet, but there was no easy way to do that. So I came up with a way,
> and this patch reflects the cleaned-up version of that way.
>
> For convenience, the following two methods are now supported ways to
> pretend that a prereq is not met:
>
> 	test_set_prereq !GPG
>
> and
>
> 	test_unset_prereq GPG

Very nice new feature, and a great description ;-)

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Published-As: https://github.com/dscho/git/releases/tag/test-unset-prereq-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git test-unset-prereq-v1
>  t/test-lib-functions.sh | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 7d620bf2a9a..76cd6630f29 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -278,8 +278,20 @@ write_script () {
>  # The single parameter is the prerequisite tag (a simple word, in all
>  # capital letters by convention).
>  
> +test_unset_prereq () {
> +	! test_have_prereq "$1" ||
> +	satisfied_prereq="${satisfied_prereq% $1 *} ${satisfied_prereq#* $1 }"
> +}
> +
>  test_set_prereq () {
> -	satisfied_prereq="$satisfied_prereq$1 "
> +	case "$1" in
> +	!*)
> +		test_unset_prereq "${1#!}"
> +		;;
> +	*)
> +		satisfied_prereq="$satisfied_prereq$1 "
> +		;;
> +	esac
>  }
>  satisfied_prereq=" "
>  lazily_testable_prereq= lazily_tested_prereq=
>
> base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d

Re: [PATCH] tests: introduce test_unset_prereq, for debugging

[SZEDER Gábor]

> While working on the --convert-graft-file test, I missed that I was
> relying on the GPG prereq, by using output of test cases that were only
> run under that prereq.

That GPG vs --convert-graft-file thing really does have a bit of a
fallout, doesn't it?  I'm at five patches and possibly counting...

> For debugging, it was really convenient to force that prereq to be
> unmet, but there was no easy way to do that.

Well, in case of the GPG prereq there is an easy way: you could have
just commented out that

  . "$TEST_DIRECTORY/lib-gpg.sh"

line near the beginning of the test script.  Just a single '#'
character, it can't get much easier than that :)

But yeah, this doesn't work with every prereq...

> So I came up with a way,
> and this patch reflects the cleaned-up version of that way.
> 
> For convenience, the following two methods are now supported ways to
> pretend that a prereq is not met:
> 
> 	test_set_prereq !GPG
> 
> and
> 
> 	test_unset_prereq GPG

I'm not sure this is the right way to do this.

I wanted to run the whole test suite with all GPG tests skipped the
other day.  With this 'test_unset_prereq' I would have to modify all
test scripts containing tests depending on the GPG prereq and add
'test_unset_prereq GPG', right?  I rather modified 't/lib-gpg.sh',
i.e. only a single file, instead of modifying the 13 test scripts that
match the 'test_expect_[a-z]*[ ,]GPG[ ,]' regexp.

I think we would be better served by an environment variable similar
to $GIT_SKIP_TESTS, e.g. $GIT_SKIP_PREREQS, to list all the prereqs
that should be skipped even if they were met.

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Published-As: https://github.com/dscho/git/releases/tag/test-unset-prereq-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git test-unset-prereq-v1
>  t/test-lib-functions.sh | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)

Anyway, please add a bit of documentation with docstrings and/or in
't/README'.

> 
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 7d620bf2a9a..76cd6630f29 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -278,8 +278,20 @@ write_script () {
>  # The single parameter is the prerequisite tag (a simple word, in all
>  # capital letters by convention).
>  
> +test_unset_prereq () {
> +	! test_have_prereq "$1" ||
> +	satisfied_prereq="${satisfied_prereq% $1 *} ${satisfied_prereq#* $1 }"
> +}
> +
>  test_set_prereq () {
> -	satisfied_prereq="$satisfied_prereq$1 "
> +	case "$1" in
> +	!*)
> +		test_unset_prereq "${1#!}"
> +		;;
> +	*)
> +		satisfied_prereq="$satisfied_prereq$1 "
> +		;;
> +	esac
>  }
>  satisfied_prereq=" "
>  lazily_testable_prereq= lazily_tested_prereq=
> 
> base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d
> -- 
> 2.17.0.windows.1.36.gdf4ca5fb72a
> 

Re: [PATCH] tests: introduce test_unset_prereq, for debugging

[Junio C Hamano]

SZEDER Gábor <szeder.dev@gmail.com> writes:

>> For convenience, the following two methods are now supported ways to
>> pretend that a prereq is not met:
>> 
>> 	test_set_prereq !GPG
>> 
>> and
>> 
>> 	test_unset_prereq GPG
>
> I'm not sure this is the right way to do this.
>
> I wanted to run the whole test suite with all GPG tests skipped the
> other day.  With this 'test_unset_prereq' I would have to modify all
> test scripts containing tests depending on the GPG prereq and add
> 'test_unset_prereq GPG', right?

Excellent point.  This won't cover that use case well.

That does not mean this change is useless.  If you are focusing on
developing a single test script, you can afford to tentatively add
these set/unset, just like you tentatively debug with echo's ;-)

But I find your SKIP_PREREQS an excellent idea.

It may want to be FORCE_PREREQS that lets you pretend a prereq is
not satisfied on your machine even when it actually is, and also
lets you pretend a prereq is satisfied on your machine even when it
is not.  SKIP_PREREQS would only do the first half, which would be
sufficient most of the time, though.

> I think we would be better served by an environment variable similar
> to $GIT_SKIP_TESTS, e.g. $GIT_SKIP_PREREQS, to list all the prereqs
> that should be skipped even if they were met.