threads / patch / 48375

patchtests: introduce test_unset_prereq, for debugging

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

## tl;dr

4 messages between Apr 28, 2018 and May 7, 2018. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Johannes Schindelin· Apr 28, 2018, 22:33 UTC · lore

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(-)
Show changes to t/test-lib-functions.sh +13 −1
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
Junio C Hamano· Apr 30, 2018, 00:38 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 16 quoted lines
> 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 ;-)
Show 35 quoted lines
> 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
SZEDER Gábor· May 7, 2018, 11:59 UTC · re: Johannes Schindelin · lore

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

> 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...
Show 11 quoted lines
> 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.

Show 6 quoted lines
> 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'.

Show 32 quoted lines
> 
> 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
> 
Junio C Hamano· May 7, 2018, 14:22 UTC · re: SZEDER Gábor · lore

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

SZEDER Gábor <szeder.dev@gmail.com> writes:
Show 15 quoted lines
>> 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.

← back to recent threads