* [PATCH] test-lib: Multi-prereq support only checked the last prereq
@ 2010-08-10 23:21 Ævar Arnfjörð Bjarmason
2010-08-11 1:43 ` Jonathan Nieder
0 siblings, 1 reply; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-10 23:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason
The support for multiple test prerequisites added by me in "test-lib:
Add support for multiple test prerequisites" was broken.
The clever for-loop (which I blindly copied from Junio) iterated over
each prerequisite and returned true/false within a case statement.
Thus only the last prerequisite in the list of prerequisites was ever
considered, the rest were ignored.
Fix that by changing the test_have_prereq code to something less
clever that keeps a count of the total prereqs and the ones we have
and compares the count at the end.
This comes with the added advantage that it's easy to list the missing
prerequisites in the test output.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
I should have spotted this earlier.
t/t0000-basic.sh | 6 +++++-
t/test-lib.sh | 21 ++++++++++++++++++---
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 2887677..9602085 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -84,7 +84,11 @@ donthaveit=yes
test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '
donthaveit=no
'
-if test $haveit$donthaveit != yesyes
+donthaveiteither=yes
+test_expect_success DONTHAVEIT,HAVEIT 'unmet prerequisites causes test to be skipped' '
+ donthaveiteither=no
+'
+if test $haveit$donthaveit$donthaveiteither != yesyesyes
then
say "bug in test framework: multiple prerequisite tags do not work reliably"
exit 1
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4e73fff..8c8e129 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -337,15 +337,30 @@ test_have_prereq () {
IFS=,
set -- $*
IFS=$save_IFS
+
+ total_prereq=0
+ ok_prereq=0
+ missing_prereq=
+
for prerequisite
do
+ total_prereq=$(($total_prereq + 1))
case $satisfied in
*" $prerequisite "*)
- : yes, have it ;;
+ ok_prereq=$(($ok_prereq + 1))
+ ;;
*)
- ! : nope ;;
+ # Keep a list of missing prerequisites
+ if test -z "$missing_prereq"
+ then
+ missing_prereq=$prerequisite
+ else
+ missing_prereq="$prerequisite,$missing_prereq"
+ fi
esac
done
+
+ test $total_prereq = $ok_prereq
}
# You are not expected to call test_ok_ and test_failure_ directly, use
@@ -408,7 +423,7 @@ test_skip () {
case "$to_skip" in
t)
say_color skip >&3 "skipping test: $@"
- say_color skip "ok $test_count # skip $1 (prereqs: $prereq)"
+ say_color skip "ok $test_count # skip $1 (missing $missing_prereq of $prereq)"
: true
;;
*)
--
1.7.2.1.295.gd03d
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] test-lib: Multi-prereq support only checked the last prereq
2010-08-10 23:21 [PATCH] test-lib: Multi-prereq support only checked the last prereq Ævar Arnfjörð Bjarmason
@ 2010-08-11 1:43 ` Jonathan Nieder
2010-08-11 12:04 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Nieder @ 2010-08-11 1:43 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano
Ævar Arnfjörð Bjarmason wrote:
> The clever for-loop (which I blindly copied from Junio)
You did not copy his “return” statement, though. :)
> +++ b/t/test-lib.sh
> @@ -337,15 +337,30 @@ test_have_prereq () {
> IFS=,
> set -- $*
> IFS=$save_IFS
> +
> + total_prereq=0
> + ok_prereq=0
> + missing_prereq=
> +
> for prerequisite
> do
> + total_prereq=$(($total_prereq + 1))
> case $satisfied in
> *" $prerequisite "*)
> - : yes, have it ;;
> + ok_prereq=$(($ok_prereq + 1))
> + ;;
> *)
> - ! : nope ;;
> + # Keep a list of missing prerequisites
> + if test -z "$missing_prereq"
> + then
> + missing_prereq=$prerequisite
> + else
> + missing_prereq="$prerequisite,$missing_prereq"
> + fi
> esac
> done
> +
> + test $total_prereq = $ok_prereq
> }
Wouldn’t
- ! : nope ;;
+ return 1 ;;
be simpler?
This and a small cleanup, including a test that catches this, are at
http://thread.gmane.org/gmane.comp.version-control.git/152814/focus=152875
Hope that helps,
Jonathan
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] test-lib: Multi-prereq support only checked the last prereq
2010-08-11 1:43 ` Jonathan Nieder
@ 2010-08-11 12:04 ` Ævar Arnfjörð Bjarmason
2010-08-13 20:28 ` Johannes Sixt
2010-08-24 7:34 ` [PATCH] tests: simplify "missing PREREQ" message Jonathan Nieder
0 siblings, 2 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-11 12:04 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jonathan Nieder, Ævar Arnfjörð Bjarmason
The support for multiple test prerequisites added by me in "test-lib:
Add support for multiple test prerequisites" was broken.
The for iterated over each prerequisite and returned true/false within
a case statement, but since it missed a return statement only the last
prerequisite in the list of prerequisites was ever considered, the
rest were ignored.
Fix that by changing the test_have_prereq code to something less
clever that keeps a count of the total prereqs and the ones we have
and compares the count at the end.
This comes with the added advantage that it's easy to list the missing
prerequisites in the test output, implement that while I'm at it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
This v2 patch has a new commit message.
On Wed, Aug 11, 2010 at 01:43, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>
>> The clever for-loop (which I blindly copied from Junio)
>
> You did not copy his “return” statement, though. :)
Oops, yeah. I copied it from some mix-match of different
patches. Corrected the patch message for that.
> Wouldn’t
>
> - ! : nope ;;
> + return 1 ;;
>
> be simpler?
Yeah, that'd make it work, but I wanted to add something that spewed a
complete listing of missing prereqs for ecah test, bailing out with a
return wouldn't do that.
t/t0000-basic.sh | 6 +++++-
t/test-lib.sh | 21 ++++++++++++++++++---
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 2887677..9602085 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -84,7 +84,11 @@ donthaveit=yes
test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '
donthaveit=no
'
-if test $haveit$donthaveit != yesyes
+donthaveiteither=yes
+test_expect_success DONTHAVEIT,HAVEIT 'unmet prerequisites causes test to be skipped' '
+ donthaveiteither=no
+'
+if test $haveit$donthaveit$donthaveiteither != yesyesyes
then
say "bug in test framework: multiple prerequisite tags do not work reliably"
exit 1
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4e73fff..8c8e129 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -337,15 +337,30 @@ test_have_prereq () {
IFS=,
set -- $*
IFS=$save_IFS
+
+ total_prereq=0
+ ok_prereq=0
+ missing_prereq=
+
for prerequisite
do
+ total_prereq=$(($total_prereq + 1))
case $satisfied in
*" $prerequisite "*)
- : yes, have it ;;
+ ok_prereq=$(($ok_prereq + 1))
+ ;;
*)
- ! : nope ;;
+ # Keep a list of missing prerequisites
+ if test -z "$missing_prereq"
+ then
+ missing_prereq=$prerequisite
+ else
+ missing_prereq="$prerequisite,$missing_prereq"
+ fi
esac
done
+
+ test $total_prereq = $ok_prereq
}
# You are not expected to call test_ok_ and test_failure_ directly, use
@@ -408,7 +423,7 @@ test_skip () {
case "$to_skip" in
t)
say_color skip >&3 "skipping test: $@"
- say_color skip "ok $test_count # skip $1 (prereqs: $prereq)"
+ say_color skip "ok $test_count # skip $1 (missing $missing_prereq of $prereq)"
: true
;;
*)
--
1.7.2.1.295.gdf931
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] test-lib: Multi-prereq support only checked the last prereq
2010-08-11 12:04 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
@ 2010-08-13 20:28 ` Johannes Sixt
2010-08-24 7:34 ` [PATCH] tests: simplify "missing PREREQ" message Jonathan Nieder
1 sibling, 0 replies; 7+ messages in thread
From: Johannes Sixt @ 2010-08-13 20:28 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Jonathan Nieder
Am 11.08.2010 14:04, schrieb Ævar Arnfjörð Bjarmason:
> - say_color skip "ok $test_count # skip $1 (prereqs: $prereq)"
> + say_color skip "ok $test_count # skip $1 (missing $missing_prereq of $prereq)"
This needs fixing: When a test is skipped due to being listed in
GIT_SKIP_TESTS, then the output looks like this:
ok 47 # skip --ignore-submodules=untracked suppresses submodules with
untracked content (missing SANITY,POSIXPERM of )
(In this case, there was a test before number 47 that was skipped due to
missing prerequisite.)
Please also note that - when a prerequisite is not satisfied - the text in
parentheses does not say "prerequisite" or similar anymore. IMO, this
should be added back.
-- Hannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] tests: simplify "missing PREREQ" message
2010-08-11 12:04 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2010-08-13 20:28 ` Johannes Sixt
@ 2010-08-24 7:34 ` Jonathan Nieder
2010-08-24 10:16 ` Johannes Sixt
2010-08-24 19:01 ` Junio C Hamano
1 sibling, 2 replies; 7+ messages in thread
From: Jonathan Nieder @ 2010-08-24 7:34 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano
When a test has no prerequisites satisfied (the usual case), instead
of "missing THING of THING", just say "missing THING". This does not
affect the output when a test is skipped due to a missing
prerequisites if another prerequisite is satisfied.
For example: instead of
ok 8 # skip notes work (missing EXPENSIVE of EXPENSIVE)
ok 9 # skip notes timing with /usr/bin/time (missing EXPENSIVE of USR_BIN_TIME,EXPENSIVE)
write
ok 8 # skip notes work (missing EXPENSIVE)
ok 9 # skip notes timing with /usr/bin/time (missing EXPENSIVE of USR_BIN_TIME,EXPENSIVE)
Cc: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Noticed this when building "next". Thoughts?
t/test-lib.sh | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4617998..c9193cb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -417,8 +417,14 @@ test_skip () {
fi
case "$to_skip" in
t)
+ of_prereq=
+ if test "$missing_prereq" != "$prereq"
+ then
+ of_prereq=" of $prereq"
+ fi
+
say_color skip >&3 "skipping test: $@"
- say_color skip "ok $test_count # skip $1 (missing $missing_prereq of $prereq)"
+ say_color skip "ok $test_count # skip $1 (missing $missing_prereq${of_prereq})"
: true
;;
*)
--
1.7.2.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tests: simplify "missing PREREQ" message
2010-08-24 7:34 ` [PATCH] tests: simplify "missing PREREQ" message Jonathan Nieder
@ 2010-08-24 10:16 ` Johannes Sixt
2010-08-24 19:01 ` Junio C Hamano
1 sibling, 0 replies; 7+ messages in thread
From: Johannes Sixt @ 2010-08-24 10:16 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ævar Arnfjörð Bjarmason, git, Junio C Hamano
Am 8/24/2010 9:34, schrieb Jonathan Nieder:
> @@ -417,8 +417,14 @@ test_skip () {
> fi
> case "$to_skip" in
> t)
> + of_prereq=
> + if test "$missing_prereq" != "$prereq"
> + then
> + of_prereq=" of $prereq"
> + fi
> +
> say_color skip >&3 "skipping test: $@"
> - say_color skip "ok $test_count # skip $1 (missing $missing_prereq of $prereq)"
> + say_color skip "ok $test_count # skip $1 (missing $missing_prereq${of_prereq})"
> : true
> ;;
> *)
While you are touching this area, would also fix messages like these:
ok 70 # skip set --path (missing of )
ok 71 # skip get --path (missing of HOMEVAR)
ok 72 # skip get --path copes with unset $HOME (missing of )
which are due to GIT_SKIP_TESTS=t1300.7[012]
-- Hannes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tests: simplify "missing PREREQ" message
2010-08-24 7:34 ` [PATCH] tests: simplify "missing PREREQ" message Jonathan Nieder
2010-08-24 10:16 ` Johannes Sixt
@ 2010-08-24 19:01 ` Junio C Hamano
1 sibling, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2010-08-24 19:01 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ævar Arnfjörð Bjarmason, git
Jonathan Nieder <jrnieder@gmail.com> writes:
> Noticed this when building "next". Thoughts?
Definitely nicer ;-)
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-08-24 19:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-10 23:21 [PATCH] test-lib: Multi-prereq support only checked the last prereq Ævar Arnfjörð Bjarmason
2010-08-11 1:43 ` Jonathan Nieder
2010-08-11 12:04 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2010-08-13 20:28 ` Johannes Sixt
2010-08-24 7:34 ` [PATCH] tests: simplify "missing PREREQ" message Jonathan Nieder
2010-08-24 10:16 ` Johannes Sixt
2010-08-24 19:01 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.