[PATCH 4/9] test: add support for external executable dependencies
On Tue, 29 Nov 2011 02:13:53 +0400, Dmitry Kurochkin wrote:
> On Tue, 29 Nov 2011 01:53:49 +0400, Dmitry Kurochkin gmail.com> wrote:
> > On Mon, 28 Nov 2011 23:16:27 +0200, Tomi Ollila
> > wrote:
> > > On Thu, 17 Nov 2011 05:56:21 +0400, Dmitry Kurochkin > > gmail.com> wrote:
> > > > There is existing support for general prerequisites in the test suite.
> > > > But it is not very convenient to use: every test case has to keep
> > > > track for it's dependencies and they have to be explicitly listed.
> > > >
> > > > The patch aims to add better support for a particular type of external
> > > > dependencies: external executables. The main idea is to replace
> > > > missing external binaries with shell functions that have the same
> > > > name. These functions always fail and keep track of missing
> > > > dependencies for a subtest. The result reporting functions later can
> > > > check that an external binaries are missing and correctly report SKIP
> > > > result instead of FAIL. The primary benefit is that the test cases do
> > > > not need to declare their dependencies or be changed in any way.
> > > > ---
> > > > test/test-lib.sh | 49
> > > > +
> > > > 1 files changed, 41 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > > > index f21e45e..ab8c6fd 100755
> > > > --- a/test/test-lib.sh
> > > > +++ b/test/test-lib.sh
> > > > @@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
> > > > # - Implicitly by specifying the prerequisite tag in the calls to
> > > > # test_expect_{success,failure,code}.
> > > > #
> > > > # The single parameter is the prerequisite tag (a simple word, in all
> > > > # capital letters by convention).
> > > >
> > > > test_set_prereq () {
> > > > satisfied="$satisfied$1 "
> > > > }
> > > > satisfied=" "
> > > >
> > > > test_have_prereq () {
> > > > case $satisfied in
> > > > *" $1 "*)
> > > > : yes, have it ;;
> > > > *)
> > > > ! : nope ;;
> > > > esac
> > > > }
> > > >
> > > > +# declare prerequisite for the given external binary
> > > > +test_declare_external_prereq () {
> > > > + binary="$1"
> > > > + test "$#" = 2 && name=$2 || name="$binary(1)"
> > > > +
> > > > +hash $binary 2>/dev/null || eval "
> > > > +$1 () {
> > > > + echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e
> > > > \" $name \" ||
> > > > +
> > > > test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_
> > > > $name\"
> > > > + false
> > > > +}"
> > > > +}
> > > > +
> > >
> > > Does this work right ?
> >
> > It does not.
> >
> > Moreover, there is a missing backslash before
> > $test_subtest_missing_external_prereqs_ (and that is why I did not
> > notice the bug during my poor testing).
> >
> > > ... the grep -e \" $name \" -- part requires
> > > spaces on both side of, but next line does not write trailing space...
> > > ... and is there leading space (and also the latest
> > > $test_subtest_missing_external_prereqs_ (just before 'false') is
> > > evaluated) ?
> > >
> > > This could perhaps be written like the above test_set_prereq &
> > > test_save_prereq:
> > > ...
> > > hash $binary 2>/dev/null || eval "
> > > $binary () {
> > > test_missing_external_prereq_${binary}_=t
> > > case \$test_subtest_missing_external_prereqs_ in
> > > *' $name '*) ;;
> > > *)
> > > test_subtest_missing_external_prereqs_=\"\$test_subtest_missing_external_prereqs_$name
> > > \"
> > > esac
> > > false
> > > }
> > >
> > > and test_subtest_missing_external_prereqs_=' ' done in test_reset_state_
> > >
> >
> > Well, this approach would obviously be better, at least because it does
> > work. But IMO it is too complex, and essentially has the same problem
> > as the current code: it mixes the check with diagnostic message.
> >
> > We already have a proper way to check if dependency is missing already:
> > test_missing_external_prereq_${binary}_. We should check it and add the
> > binary to test_subtest_missing_external_prereqs_ if it is not set. And
> > move setting of test_missing_external_prereq_${binary}_ below.
> >
> > > (I took some code from current git head also perhaps instead of
> > > setting test_missing_external_prereq_${binary}_=t, (bash) associative
> > > arrays could be taken into use -- the eval to read that information
> > > is a bit hairy -- well, at least that part works for sure :D )
> > >
> >
> > Yes! I like using hash here.
> >
> > > Hmm... how about this:
> > >
> > > hash $binary 2>/dev/null || eval "
> > > $binary () {
> > > if [ -z \"\${test_missing_external_prereq_[$binary]}\" ]
> > > then
> > > test_missing_external_prereq_[$binary]=t
> > >
> > >
[PATCH 4/9] test: add support for external executable dependencies
On Tue, 29 Nov 2011 01:53:49 +0400, Dmitry Kurochkin wrote:
> On Mon, 28 Nov 2011 23:16:27 +0200, Tomi Ollila wrote:
> > On Thu, 17 Nov 2011 05:56:21 +0400, Dmitry Kurochkin > gmail.com> wrote:
> > > There is existing support for general prerequisites in the test suite.
> > > But it is not very convenient to use: every test case has to keep
> > > track for it's dependencies and they have to be explicitly listed.
> > >
> > > The patch aims to add better support for a particular type of external
> > > dependencies: external executables. The main idea is to replace
> > > missing external binaries with shell functions that have the same
> > > name. These functions always fail and keep track of missing
> > > dependencies for a subtest. The result reporting functions later can
> > > check that an external binaries are missing and correctly report SKIP
> > > result instead of FAIL. The primary benefit is that the test cases do
> > > not need to declare their dependencies or be changed in any way.
> > > ---
> > > test/test-lib.sh | 49 +
> > > 1 files changed, 41 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > > index f21e45e..ab8c6fd 100755
> > > --- a/test/test-lib.sh
> > > +++ b/test/test-lib.sh
> > > @@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
> > > # - Implicitly by specifying the prerequisite tag in the calls to
> > > # test_expect_{success,failure,code}.
> > > #
> > > # The single parameter is the prerequisite tag (a simple word, in all
> > > # capital letters by convention).
> > >
> > > test_set_prereq () {
> > > satisfied="$satisfied$1 "
> > > }
> > > satisfied=" "
> > >
> > > test_have_prereq () {
> > > case $satisfied in
> > > *" $1 "*)
> > > : yes, have it ;;
> > > *)
> > > ! : nope ;;
> > > esac
> > > }
> > >
> > > +# declare prerequisite for the given external binary
> > > +test_declare_external_prereq () {
> > > + binary="$1"
> > > + test "$#" = 2 && name=$2 || name="$binary(1)"
> > > +
> > > +hash $binary 2>/dev/null || eval "
> > > +$1 () {
> > > + echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name
> > > \" ||
> > > +
> > > test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_
> > > $name\"
> > > + false
> > > +}"
> > > +}
> > > +
> >
> > Does this work right ?
>
> It does not.
>
> Moreover, there is a missing backslash before
> $test_subtest_missing_external_prereqs_ (and that is why I did not
> notice the bug during my poor testing).
>
> > ... the grep -e \" $name \" -- part requires
> > spaces on both side of, but next line does not write trailing space...
> > ... and is there leading space (and also the latest
> > $test_subtest_missing_external_prereqs_ (just before 'false') is evaluated)
> > ?
> >
> > This could perhaps be written like the above test_set_prereq &
> > test_save_prereq:
> > ...
> > hash $binary 2>/dev/null || eval "
> > $binary () {
> > test_missing_external_prereq_${binary}_=t
> > case \$test_subtest_missing_external_prereqs_ in
> > *' $name '*) ;;
> > *)
> > test_subtest_missing_external_prereqs_=\"\$test_subtest_missing_external_prereqs_$name
> > \"
> > esac
> > false
> > }
> >
> > and test_subtest_missing_external_prereqs_=' ' done in test_reset_state_
> >
>
> Well, this approach would obviously be better, at least because it does
> work. But IMO it is too complex, and essentially has the same problem
> as the current code: it mixes the check with diagnostic message.
>
> We already have a proper way to check if dependency is missing already:
> test_missing_external_prereq_${binary}_. We should check it and add the
> binary to test_subtest_missing_external_prereqs_ if it is not set. And
> move setting of test_missing_external_prereq_${binary}_ below.
>
> > (I took some code from current git head also perhaps instead of
> > setting test_missing_external_prereq_${binary}_=t, (bash) associative
> > arrays could be taken into use -- the eval to read that information
> > is a bit hairy -- well, at least that part works for sure :D )
> >
>
> Yes! I like using hash here.
>
> > Hmm... how about this:
> >
> > hash $binary 2>/dev/null || eval "
> > $binary () {
> > if [ -z \"\${test_missing_external_prereq_[$binary]}\" ]
> > then
> > test_missing_external_prereq_[$binary]=t
> >
> > test_subtest_missing_external_prereqs_=\"\$test_subtest_missing_external_prereqs_
> > $name\"
> > fi
> > false
> > }
> >
>
> There is some inconsistent indenting in the above code (mixed tabs and
> spaces). Also test_require_external_prereq() should be changed as well.
>
> Otherwise I like it. Would you please submit a patch?
>
Actually, test_missing_external_prereq_${binary}_ must be set outside of
the $binary() function. It
[PATCH 4/9] test: add support for external executable dependencies
On Mon, 28 Nov 2011 23:16:27 +0200, Tomi Ollila wrote:
> On Thu, 17 Nov 2011 05:56:21 +0400, Dmitry Kurochkin gmail.com> wrote:
> > There is existing support for general prerequisites in the test suite.
> > But it is not very convenient to use: every test case has to keep
> > track for it's dependencies and they have to be explicitly listed.
> >
> > The patch aims to add better support for a particular type of external
> > dependencies: external executables. The main idea is to replace
> > missing external binaries with shell functions that have the same
> > name. These functions always fail and keep track of missing
> > dependencies for a subtest. The result reporting functions later can
> > check that an external binaries are missing and correctly report SKIP
> > result instead of FAIL. The primary benefit is that the test cases do
> > not need to declare their dependencies or be changed in any way.
> > ---
> > test/test-lib.sh | 49 +
> > 1 files changed, 41 insertions(+), 8 deletions(-)
> >
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index f21e45e..ab8c6fd 100755
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
> > # - Implicitly by specifying the prerequisite tag in the calls to
> > # test_expect_{success,failure,code}.
> > #
> > # The single parameter is the prerequisite tag (a simple word, in all
> > # capital letters by convention).
> >
> > test_set_prereq () {
> > satisfied="$satisfied$1 "
> > }
> > satisfied=" "
> >
> > test_have_prereq () {
> > case $satisfied in
> > *" $1 "*)
> > : yes, have it ;;
> > *)
> > ! : nope ;;
> > esac
> > }
> >
> > +# declare prerequisite for the given external binary
> > +test_declare_external_prereq () {
> > + binary="$1"
> > + test "$#" = 2 && name=$2 || name="$binary(1)"
> > +
> > +hash $binary 2>/dev/null || eval "
> > +$1 () {
> > + echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name
> > \" ||
> > +
> > test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_
> > $name\"
> > + false
> > +}"
> > +}
> > +
>
> Does this work right ?
It does not.
Moreover, there is a missing backslash before
$test_subtest_missing_external_prereqs_ (and that is why I did not
notice the bug during my poor testing).
> ... the grep -e \" $name \" -- part requires
> spaces on both side of, but next line does not write trailing space...
> ... and is there leading space (and also the latest
> $test_subtest_missing_external_prereqs_ (just before 'false') is evaluated) ?
>
> This could perhaps be written like the above test_set_prereq &
> test_save_prereq:
> ...
> hash $binary 2>/dev/null || eval "
> $binary () {
> test_missing_external_prereq_${binary}_=t
> case \$test_subtest_missing_external_prereqs_ in
> *' $name '*) ;;
> *)
> test_subtest_missing_external_prereqs_=\"\$test_subtest_missing_external_prereqs_$name
> \"
> esac
> false
> }
>
> and test_subtest_missing_external_prereqs_=' ' done in test_reset_state_
>
Well, this approach would obviously be better, at least because it does
work. But IMO it is too complex, and essentially has the same problem
as the current code: it mixes the check with diagnostic message.
We already have a proper way to check if dependency is missing already:
test_missing_external_prereq_${binary}_. We should check it and add the
binary to test_subtest_missing_external_prereqs_ if it is not set. And
move setting of test_missing_external_prereq_${binary}_ below.
> (I took some code from current git head also perhaps instead of
> setting test_missing_external_prereq_${binary}_=t, (bash) associative
> arrays could be taken into use -- the eval to read that information
> is a bit hairy -- well, at least that part works for sure :D )
>
Yes! I like using hash here.
> Hmm... how about this:
>
> hash $binary 2>/dev/null || eval "
> $binary () {
> if [ -z \"\${test_missing_external_prereq_[$binary]}\" ]
> then
> test_missing_external_prereq_[$binary]=t
>
> test_subtest_missing_external_prereqs_=\"\$test_subtest_missing_external_prereqs_
> $name\"
> fi
> false
> }
>
There is some inconsistent indenting in the above code (mixed tabs and
spaces). Also test_require_external_prereq() should be changed as well.
Otherwise I like it. Would you please submit a patch?
Regards,
Dmitry
> and test_subtest_missing_external_prereqs_ cleared in test_reset_state_ like
> now.
>
> Tomi
Re: [PATCH 4/9] test: add support for external executable dependencies
On Thu, 17 Nov 2011 05:56:21 +0400, Dmitry Kurochkin
dmitry.kuroch...@gmail.com wrote:
There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.
The patch aims to add better support for a particular type of external
dependencies: external executables. The main idea is to replace
missing external binaries with shell functions that have the same
name. These functions always fail and keep track of missing
dependencies for a subtest. The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL. The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
test/test-lib.sh | 49 +
1 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f21e45e..ab8c6fd 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
# - Implicitly by specifying the prerequisite tag in the calls to
# test_expect_{success,failure,code}.
#
# The single parameter is the prerequisite tag (a simple word, in all
# capital letters by convention).
test_set_prereq () {
satisfied=$satisfied$1
}
satisfied=
test_have_prereq () {
case $satisfied in
* $1 *)
: yes, have it ;;
*)
! : nope ;;
esac
}
+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+ binary=$1
+ test $# = 2 name=$2 || name=$binary(1)
+
+hash $binary 2/dev/null || eval
+$1 () {
+ echo -n \\$test_subtest_missing_external_prereqs_\ | grep -e \ $name
\ ||
+
test_subtest_missing_external_prereqs_=\$test_subtest_missing_external_prereqs_
$name\
+ false
+}
+}
+
Does this work right ? ... the grep -e \ $name \ -- part requires
spaces on both side of, but next line does not write trailing space...
... and is there leading space (and also the latest
$test_subtest_missing_external_prereqs_ (just before 'false') is evaluated) ?
This could perhaps be written like the above test_set_prereq
test_save_prereq:
...
hash $binary 2/dev/null || eval
$binary () {
test_missing_external_prereq_${binary}_=t
case \$test_subtest_missing_external_prereqs_ in
*' $name '*) ;;
*)
test_subtest_missing_external_prereqs_=\\$test_subtest_missing_external_prereqs_$name
\
esac
false
}
and test_subtest_missing_external_prereqs_=' ' done in test_reset_state_
(I took some code from current git head also perhaps instead of
setting test_missing_external_prereq_${binary}_=t, (bash) associative
arrays could be taken into use -- the eval to read that information
is a bit hairy -- well, at least that part works for sure :D )
Hmm... how about this:
hash $binary 2/dev/null || eval
$binary () {
if [ -z \\${test_missing_external_prereq_[$binary]}\ ]
then
test_missing_external_prereq_[$binary]=t
test_subtest_missing_external_prereqs_=\\$test_subtest_missing_external_prereqs_
$name\
fi
false
}
and test_subtest_missing_external_prereqs_ cleared in test_reset_state_ like
now.
Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/9] test: add support for external executable dependencies
On Mon, 28 Nov 2011 23:16:27 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
On Thu, 17 Nov 2011 05:56:21 +0400, Dmitry Kurochkin
dmitry.kuroch...@gmail.com wrote:
There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.
The patch aims to add better support for a particular type of external
dependencies: external executables. The main idea is to replace
missing external binaries with shell functions that have the same
name. These functions always fail and keep track of missing
dependencies for a subtest. The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL. The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
test/test-lib.sh | 49 +
1 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f21e45e..ab8c6fd 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
# - Implicitly by specifying the prerequisite tag in the calls to
# test_expect_{success,failure,code}.
#
# The single parameter is the prerequisite tag (a simple word, in all
# capital letters by convention).
test_set_prereq () {
satisfied=$satisfied$1
}
satisfied=
test_have_prereq () {
case $satisfied in
* $1 *)
: yes, have it ;;
*)
! : nope ;;
esac
}
+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+ binary=$1
+ test $# = 2 name=$2 || name=$binary(1)
+
+hash $binary 2/dev/null || eval
+$1 () {
+ echo -n \\$test_subtest_missing_external_prereqs_\ | grep -e \ $name
\ ||
+
test_subtest_missing_external_prereqs_=\$test_subtest_missing_external_prereqs_
$name\
+ false
+}
+}
+
Does this work right ?
It does not.
Moreover, there is a missing backslash before
$test_subtest_missing_external_prereqs_ (and that is why I did not
notice the bug during my poor testing).
... the grep -e \ $name \ -- part requires
spaces on both side of, but next line does not write trailing space...
... and is there leading space (and also the latest
$test_subtest_missing_external_prereqs_ (just before 'false') is evaluated) ?
This could perhaps be written like the above test_set_prereq
test_save_prereq:
...
hash $binary 2/dev/null || eval
$binary () {
test_missing_external_prereq_${binary}_=t
case \$test_subtest_missing_external_prereqs_ in
*' $name '*) ;;
*)
test_subtest_missing_external_prereqs_=\\$test_subtest_missing_external_prereqs_$name
\
esac
false
}
and test_subtest_missing_external_prereqs_=' ' done in test_reset_state_
Well, this approach would obviously be better, at least because it does
work. But IMO it is too complex, and essentially has the same problem
as the current code: it mixes the check with diagnostic message.
We already have a proper way to check if dependency is missing already:
test_missing_external_prereq_${binary}_. We should check it and add the
binary to test_subtest_missing_external_prereqs_ if it is not set. And
move setting of test_missing_external_prereq_${binary}_ below.
(I took some code from current git head also perhaps instead of
setting test_missing_external_prereq_${binary}_=t, (bash) associative
arrays could be taken into use -- the eval to read that information
is a bit hairy -- well, at least that part works for sure :D )
Yes! I like using hash here.
Hmm... how about this:
hash $binary 2/dev/null || eval
$binary () {
if [ -z \\${test_missing_external_prereq_[$binary]}\ ]
then
test_missing_external_prereq_[$binary]=t
test_subtest_missing_external_prereqs_=\\$test_subtest_missing_external_prereqs_
$name\
fi
false
}
There is some inconsistent indenting in the above code (mixed tabs and
spaces). Also test_require_external_prereq() should be changed as well.
Otherwise I like it. Would you please submit a patch?
Regards,
Dmitry
and test_subtest_missing_external_prereqs_ cleared in test_reset_state_ like
now.
Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/9] test: add support for external executable dependencies
On Tue, 29 Nov 2011 01:53:49 +0400, Dmitry Kurochkin
dmitry.kuroch...@gmail.com wrote:
On Mon, 28 Nov 2011 23:16:27 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
On Thu, 17 Nov 2011 05:56:21 +0400, Dmitry Kurochkin
dmitry.kuroch...@gmail.com wrote:
There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.
The patch aims to add better support for a particular type of external
dependencies: external executables. The main idea is to replace
missing external binaries with shell functions that have the same
name. These functions always fail and keep track of missing
dependencies for a subtest. The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL. The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
test/test-lib.sh | 49 +
1 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f21e45e..ab8c6fd 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
# - Implicitly by specifying the prerequisite tag in the calls to
# test_expect_{success,failure,code}.
#
# The single parameter is the prerequisite tag (a simple word, in all
# capital letters by convention).
test_set_prereq () {
satisfied=$satisfied$1
}
satisfied=
test_have_prereq () {
case $satisfied in
* $1 *)
: yes, have it ;;
*)
! : nope ;;
esac
}
+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+ binary=$1
+ test $# = 2 name=$2 || name=$binary(1)
+
+hash $binary 2/dev/null || eval
+$1 () {
+ echo -n \\$test_subtest_missing_external_prereqs_\ | grep -e \ $name
\ ||
+
test_subtest_missing_external_prereqs_=\$test_subtest_missing_external_prereqs_
$name\
+ false
+}
+}
+
Does this work right ?
It does not.
Moreover, there is a missing backslash before
$test_subtest_missing_external_prereqs_ (and that is why I did not
notice the bug during my poor testing).
... the grep -e \ $name \ -- part requires
spaces on both side of, but next line does not write trailing space...
... and is there leading space (and also the latest
$test_subtest_missing_external_prereqs_ (just before 'false') is evaluated)
?
This could perhaps be written like the above test_set_prereq
test_save_prereq:
...
hash $binary 2/dev/null || eval
$binary () {
test_missing_external_prereq_${binary}_=t
case \$test_subtest_missing_external_prereqs_ in
*' $name '*) ;;
*)
test_subtest_missing_external_prereqs_=\\$test_subtest_missing_external_prereqs_$name
\
esac
false
}
and test_subtest_missing_external_prereqs_=' ' done in test_reset_state_
Well, this approach would obviously be better, at least because it does
work. But IMO it is too complex, and essentially has the same problem
as the current code: it mixes the check with diagnostic message.
We already have a proper way to check if dependency is missing already:
test_missing_external_prereq_${binary}_. We should check it and add the
binary to test_subtest_missing_external_prereqs_ if it is not set. And
move setting of test_missing_external_prereq_${binary}_ below.
(I took some code from current git head also perhaps instead of
setting test_missing_external_prereq_${binary}_=t, (bash) associative
arrays could be taken into use -- the eval to read that information
is a bit hairy -- well, at least that part works for sure :D )
Yes! I like using hash here.
Hmm... how about this:
hash $binary 2/dev/null || eval
$binary () {
if [ -z \\${test_missing_external_prereq_[$binary]}\ ]
then
test_missing_external_prereq_[$binary]=t
test_subtest_missing_external_prereqs_=\\$test_subtest_missing_external_prereqs_
$name\
fi
false
}
There is some inconsistent indenting in the above code (mixed tabs and
spaces). Also test_require_external_prereq() should be changed as well.
Otherwise I like it. Would you please submit a patch?
Actually, test_missing_external_prereq_${binary}_ must be set outside of
the $binary() function. It indicates that the binary is missing, not
that it is required. I will send some patches to fix these bugs. Then
you can change it to use hashes.
Regards,
Dmitry
Regards,
Dmitry
and test_subtest_missing_external_prereqs_ cleared in test_reset_state_
Re: [PATCH 4/9] test: add support for external executable dependencies
On Tue, 29 Nov 2011 02:13:53 +0400, Dmitry Kurochkin
dmitry.kuroch...@gmail.com wrote:
On Tue, 29 Nov 2011 01:53:49 +0400, Dmitry Kurochkin
dmitry.kuroch...@gmail.com wrote:
On Mon, 28 Nov 2011 23:16:27 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
On Thu, 17 Nov 2011 05:56:21 +0400, Dmitry Kurochkin
dmitry.kuroch...@gmail.com wrote:
There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.
The patch aims to add better support for a particular type of external
dependencies: external executables. The main idea is to replace
missing external binaries with shell functions that have the same
name. These functions always fail and keep track of missing
dependencies for a subtest. The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL. The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
test/test-lib.sh | 49
+
1 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f21e45e..ab8c6fd 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
# - Implicitly by specifying the prerequisite tag in the calls to
# test_expect_{success,failure,code}.
#
# The single parameter is the prerequisite tag (a simple word, in all
# capital letters by convention).
test_set_prereq () {
satisfied=$satisfied$1
}
satisfied=
test_have_prereq () {
case $satisfied in
* $1 *)
: yes, have it ;;
*)
! : nope ;;
esac
}
+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+ binary=$1
+ test $# = 2 name=$2 || name=$binary(1)
+
+hash $binary 2/dev/null || eval
+$1 () {
+ echo -n \\$test_subtest_missing_external_prereqs_\ | grep -e
\ $name \ ||
+
test_subtest_missing_external_prereqs_=\$test_subtest_missing_external_prereqs_
$name\
+ false
+}
+}
+
Does this work right ?
It does not.
Moreover, there is a missing backslash before
$test_subtest_missing_external_prereqs_ (and that is why I did not
notice the bug during my poor testing).
... the grep -e \ $name \ -- part requires
spaces on both side of, but next line does not write trailing space...
... and is there leading space (and also the latest
$test_subtest_missing_external_prereqs_ (just before 'false') is
evaluated) ?
This could perhaps be written like the above test_set_prereq
test_save_prereq:
...
hash $binary 2/dev/null || eval
$binary () {
test_missing_external_prereq_${binary}_=t
case \$test_subtest_missing_external_prereqs_ in
*' $name '*) ;;
*)
test_subtest_missing_external_prereqs_=\\$test_subtest_missing_external_prereqs_$name
\
esac
false
}
and test_subtest_missing_external_prereqs_=' ' done in test_reset_state_
Well, this approach would obviously be better, at least because it does
work. But IMO it is too complex, and essentially has the same problem
as the current code: it mixes the check with diagnostic message.
We already have a proper way to check if dependency is missing already:
test_missing_external_prereq_${binary}_. We should check it and add the
binary to test_subtest_missing_external_prereqs_ if it is not set. And
move setting of test_missing_external_prereq_${binary}_ below.
(I took some code from current git head also perhaps instead of
setting test_missing_external_prereq_${binary}_=t, (bash) associative
arrays could be taken into use -- the eval to read that information
is a bit hairy -- well, at least that part works for sure :D )
Yes! I like using hash here.
Hmm... how about this:
hash $binary 2/dev/null || eval
$binary () {
if [ -z \\${test_missing_external_prereq_[$binary]}\ ]
then
test_missing_external_prereq_[$binary]=t
test_subtest_missing_external_prereqs_=\\$test_subtest_missing_external_prereqs_
$name\
fi
false
}
There is some inconsistent indenting in the above code (mixed tabs and
spaces). Also test_require_external_prereq() should be changed as well.
Otherwise I like it. Would you please submit a patch?
Actually, test_missing_external_prereq_${binary}_ must be set outside of
the
[PATCH 4/9] test: add support for external executable dependencies
On Thu, 17 Nov 2011 05:56:21 +0400, Dmitry Kurochkin wrote:
> There is existing support for general prerequisites in the test suite.
> But it is not very convenient to use: every test case has to keep
> track for it's dependencies and they have to be explicitly listed.
>
> The patch aims to add better support for a particular type of external
> dependencies: external executables. The main idea is to replace
> missing external binaries with shell functions that have the same
> name. These functions always fail and keep track of missing
> dependencies for a subtest. The result reporting functions later can
> check that an external binaries are missing and correctly report SKIP
> result instead of FAIL. The primary benefit is that the test cases do
> not need to declare their dependencies or be changed in any way.
> ---
> test/test-lib.sh | 49 +
> 1 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index f21e45e..ab8c6fd 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
> # - Implicitly by specifying the prerequisite tag in the calls to
> # test_expect_{success,failure,code}.
> #
> # The single parameter is the prerequisite tag (a simple word, in all
> # capital letters by convention).
>
> test_set_prereq () {
> satisfied="$satisfied$1 "
> }
> satisfied=" "
>
> test_have_prereq () {
> case $satisfied in
> *" $1 "*)
> : yes, have it ;;
> *)
> ! : nope ;;
> esac
> }
>
> +# declare prerequisite for the given external binary
> +test_declare_external_prereq () {
> + binary="$1"
> + test "$#" = 2 && name=$2 || name="$binary(1)"
> +
> +hash $binary 2>/dev/null || eval "
> +$1 () {
> + echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name
> \" ||
> +
> test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_
> $name\"
> + false
> +}"
> +}
> +
Does this work right ? ... the grep -e \" $name \" -- part requires
spaces on both side of, but next line does not write trailing space...
... and is there leading space (and also the latest
$test_subtest_missing_external_prereqs_ (just before 'false') is evaluated) ?
This could perhaps be written like the above test_set_prereq &
test_save_prereq:
...
hash $binary 2>/dev/null || eval "
$binary () {
test_missing_external_prereq_${binary}_=t
case \$test_subtest_missing_external_prereqs_ in
*' $name '*) ;;
*)
test_subtest_missing_external_prereqs_=\"\$test_subtest_missing_external_prereqs_$name
\"
esac
false
}
and test_subtest_missing_external_prereqs_=' ' done in test_reset_state_
(I took some code from current git head also perhaps instead of
setting test_missing_external_prereq_${binary}_=t, (bash) associative
arrays could be taken into use -- the eval to read that information
is a bit hairy -- well, at least that part works for sure :D )
Hmm... how about this:
hash $binary 2>/dev/null || eval "
$binary () {
if [ -z \"\${test_missing_external_prereq_[$binary]}\" ]
then
test_missing_external_prereq_[$binary]=t
test_subtest_missing_external_prereqs_=\"\$test_subtest_missing_external_prereqs_
$name\"
fi
false
}
and test_subtest_missing_external_prereqs_ cleared in test_reset_state_ like
now.
Tomi
[PATCH 4/9] test: add support for external executable dependencies
There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.
The patch aims to add better support for a particular type of external
dependencies: external executables. The main idea is to replace
missing external binaries with shell functions that have the same
name. These functions always fail and keep track of missing
dependencies for a subtest. The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL. The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
test/test-lib.sh | 49 +
1 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f21e45e..ab8c6fd 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
# - Implicitly by specifying the prerequisite tag in the calls to
# test_expect_{success,failure,code}.
#
# The single parameter is the prerequisite tag (a simple word, in all
# capital letters by convention).
test_set_prereq () {
satisfied="$satisfied$1 "
}
satisfied=" "
test_have_prereq () {
case $satisfied in
*" $1 "*)
: yes, have it ;;
*)
! : nope ;;
esac
}
+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+ binary="$1"
+ test "$#" = 2 && name=$2 || name="$binary(1)"
+
+hash $binary 2>/dev/null || eval "
+$1 () {
+ echo -n \"\$test_subtest_missing_external_prereqs_\" | grep -e \" $name
\" ||
+
test_subtest_missing_external_prereqs_=\"$test_subtest_missing_external_prereqs_
$name\"
+ false
+}"
+}
+
# You are not expected to call test_ok_ and test_failure_ directly, use
# the text_expect_* functions instead.
test_ok_ () {
if test "$test_subtest_known_broken_" = "t"; then
test_known_broken_ok_ "$@"
return
fi
test_success=$(($test_success + 1))
say_color pass "%-6s" "PASS"
echo " $@"
}
test_failure_ () {
if test "$test_subtest_known_broken_" = "t"; then
test_known_broken_failure_ "$@"
return
fi
test_failure=$(($test_failure + 1))
test_failure_message_ "FAIL" "$@"
@@ -602,82 +615,101 @@ test_run_ () {
return 0
}
test_skip () {
test_count=$(($test_count+1))
to_skip=
for skp in $NOTMUCH_SKIP_TESTS
do
case $this_test.$test_count in
$skp)
to_skip=t
esac
done
if test -z "$to_skip" && test -n "$prereq" &&
! test_have_prereq "$prereq"
then
to_skip=t
fi
case "$to_skip" in
t)
- test_reset_state_
- say_color skip >&3 "skipping test: $@"
- say_color skip "%-6s" "SKIP"
- echo " $1"
- : true
+ test_report_skip_ "$@"
;;
*)
- false
+ test_check_missing_external_prereqs_ "$@"
;;
esac
}
+test_check_missing_external_prereqs_ () {
+ if test -n "$test_subtest_missing_external_prereqs_"; then
+ say_color skip >&3 "missing prerequisites:"
+ echo "$test_subtest_missing_external_prereqs_" >&3
+ test_report_skip_ "$@"
+ else
+ false
+ fi
+}
+
+test_report_skip_ () {
+ test_reset_state_
+ say_color skip >&3 "skipping test: $@"
+ say_color skip "%-6s" "SKIP"
+ echo " $1"
+}
+
test_subtest_known_broken () {
test_subtest_known_broken_=t
}
test_expect_success () {
test "$#" = 3 && { prereq=$1; shift; } || prereq=
test "$#" = 2 ||
error "bug in the test script: not 2 or 3 parameters to
test-expect-success"
test_reset_state_
if ! test_skip "$@"
then
test_run_ "$2"
- if [ "$?" = 0 -a "$eval_ret" = 0 ]
+ run_ret="$?"
+ # test_run_ may update missing external prerequisites
+ test_check_missing_external_prereqs_ "$@" ||
+ if [ "$run_ret" = 0 -a "$eval_ret" = 0 ]
then
test_ok_ "$1"
else
test_failure_ "$@"
fi
fi
}
test_expect_code () {
test "$#" = 4 && { prereq=$1; shift; } || prereq=
test "$#" = 3 ||
error "bug in the test script: not 3 or 4 parameters to
test-expect-code"
test_reset_state_
if ! test_skip "$@"
then
test_run_ "$3"
- if [
[PATCH 4/9] test: add support for external executable dependencies
There is existing support for general prerequisites in the test suite.
But it is not very convenient to use: every test case has to keep
track for it's dependencies and they have to be explicitly listed.
The patch aims to add better support for a particular type of external
dependencies: external executables. The main idea is to replace
missing external binaries with shell functions that have the same
name. These functions always fail and keep track of missing
dependencies for a subtest. The result reporting functions later can
check that an external binaries are missing and correctly report SKIP
result instead of FAIL. The primary benefit is that the test cases do
not need to declare their dependencies or be changed in any way.
---
test/test-lib.sh | 49 +
1 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f21e45e..ab8c6fd 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -526,40 +526,53 @@ notmuch_json_show_sanitize ()
# - Implicitly by specifying the prerequisite tag in the calls to
# test_expect_{success,failure,code}.
#
# The single parameter is the prerequisite tag (a simple word, in all
# capital letters by convention).
test_set_prereq () {
satisfied=$satisfied$1
}
satisfied=
test_have_prereq () {
case $satisfied in
* $1 *)
: yes, have it ;;
*)
! : nope ;;
esac
}
+# declare prerequisite for the given external binary
+test_declare_external_prereq () {
+ binary=$1
+ test $# = 2 name=$2 || name=$binary(1)
+
+hash $binary 2/dev/null || eval
+$1 () {
+ echo -n \\$test_subtest_missing_external_prereqs_\ | grep -e \ $name
\ ||
+
test_subtest_missing_external_prereqs_=\$test_subtest_missing_external_prereqs_
$name\
+ false
+}
+}
+
# You are not expected to call test_ok_ and test_failure_ directly, use
# the text_expect_* functions instead.
test_ok_ () {
if test $test_subtest_known_broken_ = t; then
test_known_broken_ok_ $@
return
fi
test_success=$(($test_success + 1))
say_color pass %-6s PASS
echo $@
}
test_failure_ () {
if test $test_subtest_known_broken_ = t; then
test_known_broken_failure_ $@
return
fi
test_failure=$(($test_failure + 1))
test_failure_message_ FAIL $@
@@ -602,82 +615,101 @@ test_run_ () {
return 0
}
test_skip () {
test_count=$(($test_count+1))
to_skip=
for skp in $NOTMUCH_SKIP_TESTS
do
case $this_test.$test_count in
$skp)
to_skip=t
esac
done
if test -z $to_skip test -n $prereq
! test_have_prereq $prereq
then
to_skip=t
fi
case $to_skip in
t)
- test_reset_state_
- say_color skip 3 skipping test: $@
- say_color skip %-6s SKIP
- echo $1
- : true
+ test_report_skip_ $@
;;
*)
- false
+ test_check_missing_external_prereqs_ $@
;;
esac
}
+test_check_missing_external_prereqs_ () {
+ if test -n $test_subtest_missing_external_prereqs_; then
+ say_color skip 3 missing prerequisites:
+ echo $test_subtest_missing_external_prereqs_ 3
+ test_report_skip_ $@
+ else
+ false
+ fi
+}
+
+test_report_skip_ () {
+ test_reset_state_
+ say_color skip 3 skipping test: $@
+ say_color skip %-6s SKIP
+ echo $1
+}
+
test_subtest_known_broken () {
test_subtest_known_broken_=t
}
test_expect_success () {
test $# = 3 { prereq=$1; shift; } || prereq=
test $# = 2 ||
error bug in the test script: not 2 or 3 parameters to
test-expect-success
test_reset_state_
if ! test_skip $@
then
test_run_ $2
- if [ $? = 0 -a $eval_ret = 0 ]
+ run_ret=$?
+ # test_run_ may update missing external prerequisites
+ test_check_missing_external_prereqs_ $@ ||
+ if [ $run_ret = 0 -a $eval_ret = 0 ]
then
test_ok_ $1
else
test_failure_ $@
fi
fi
}
test_expect_code () {
test $# = 4 { prereq=$1; shift; } || prereq=
test $# = 3 ||
error bug in the test script: not 3 or 4 parameters to
test-expect-code
test_reset_state_
if ! test_skip $@
then
test_run_ $3
- if [ $? = 0 -a $eval_ret = $1 ]
+ run_ret=$?
+ # test_run_ may update missing external prerequisites,
+
