Thread: how to handle missing "prove"
Here is a patch to use "missing" to handle the case when "prove" is not present. Other ideas?
Attachment
Peter Eisentraut <peter_e@gmx.net> writes:
> Here is a patch to use "missing" to handle the case when "prove" is not
> present.
Wouldn't it be easier to do what we do for Perl, viz in Makefile.global.in
ifneq (@PERL@,) # quoted to protect pathname with spaces PERL = '@PERL@'
else PERL = $(missing) perl
endif
However, with either of these approaches, "make check-world" gets a hard
failure if you lack "prove". Is that what we want? It's certainly not
very consistent with what you've been doing to make the tests just slide
by (rather than fail on) missing/too old Perl modules.
ISTM that the project policy for external components like this has been
"don't rely on them unless user says to use them, in which case fail if
they aren't present". So perhaps what we ought to have is a configure
switch along the lines of "--enable-tap-tests". If you don't specify it,
prove_check expands to nothing. If you do specify it, we fail if we
lack any of the expected support, both "prove" and whatever the agreed-on
set of Perl modules is.
regards, tom lane
On 10/28/14 9:16 PM, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> Here is a patch to use "missing" to handle the case when "prove" is not >> present. > > Wouldn't it be easier to do what we do for Perl, viz in Makefile.global.in > > ifneq (@PERL@,) > # quoted to protect pathname with spaces > PERL = '@PERL@' > else > PERL = $(missing) perl > endif Yeah, maybe. > However, with either of these approaches, "make check-world" gets a hard > failure if you lack "prove". Is that what we want? It's certainly not > very consistent with what you've been doing to make the tests just slide > by (rather than fail on) missing/too old Perl modules. The patch has -$(missing) prove and the - will make make ignore failures. Admittedly, that is very well hidden. > ISTM that the project policy for external components like this has been > "don't rely on them unless user says to use them, in which case fail if > they aren't present". So perhaps what we ought to have is a configure > switch along the lines of "--enable-tap-tests". If you don't specify it, > prove_check expands to nothing. If you do specify it, we fail if we > lack any of the expected support, both "prove" and whatever the agreed-on > set of Perl modules is. That's also a good idea. (I might think of a different option name, because "TAP" is an output format, not a piece of software. pg_regress could output TAP as well, for example.)
On 10/28/2014 09:16 PM, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> Here is a patch to use "missing" to handle the case when "prove" is not >> present. > Wouldn't it be easier to do what we do for Perl, viz in Makefile.global.in > > ifneq (@PERL@,) > # quoted to protect pathname with spaces > PERL = '@PERL@' > else > PERL = $(missing) perl > endif > > However, with either of these approaches, "make check-world" gets a hard > failure if you lack "prove". Is that what we want? It's certainly not > very consistent with what you've been doing to make the tests just slide > by (rather than fail on) missing/too old Perl modules. > > ISTM that the project policy for external components like this has been > "don't rely on them unless user says to use them, in which case fail if > they aren't present". So perhaps what we ought to have is a configure > switch along the lines of "--enable-tap-tests". If you don't specify it, > prove_check expands to nothing. If you do specify it, we fail if we > lack any of the expected support, both "prove" and whatever the agreed-on > set of Perl modules is. > > +1 If we go this way I'll add a tap icon to the buildfarm so you can see which animals are running the tests. cheers andrew
On 10/28/14 10:01 PM, Peter Eisentraut wrote: > On 10/28/14 9:16 PM, Tom Lane wrote: >> ISTM that the project policy for external components like this has been >> "don't rely on them unless user says to use them, in which case fail if >> they aren't present". So perhaps what we ought to have is a configure >> switch along the lines of "--enable-tap-tests". If you don't specify it, >> prove_check expands to nothing. If you do specify it, we fail if we >> lack any of the expected support, both "prove" and whatever the agreed-on >> set of Perl modules is. > > That's also a good idea. Here is a patch.
Attachment
Peter Eisentraut <peter_e@gmx.net> writes:
> On 10/28/14 10:01 PM, Peter Eisentraut wrote:
>> On 10/28/14 9:16 PM, Tom Lane wrote:
>>> ISTM that the project policy for external components like this has been
>>> "don't rely on them unless user says to use them, in which case fail if
>>> they aren't present". So perhaps what we ought to have is a configure
>>> switch along the lines of "--enable-tap-tests". If you don't specify it,
>>> prove_check expands to nothing. If you do specify it, we fail if we
>>> lack any of the expected support, both "prove" and whatever the agreed-on
>>> set of Perl modules is.
>> That's also a good idea.
> Here is a patch.
Looks generally reasonable, but I thought you were planning to choose a
different option name?
One minor nitpick: perhaps the --help description of the option should
read
+ --enable-tap-tests enable TAP tests (requires Perl and IPC::Run)
because in practice it'll be much more likely that people will be missing
IPC::Run than that they'll be missing Perl altogether.
Also, shouldn't we have it fail rather than just skipping tests if
IPC::Run is missing?
regards, tom lane
On 10/30/14 9:09 PM, Tom Lane wrote: > Looks generally reasonable, but I thought you were planning to choose a > different option name? Yeah, but I couldn't think of a better one. (Anything involving, "enable-perl-..." would have been confusing with regard to PL/Perl.) > One minor nitpick: perhaps the --help description of the option should > read > > + --enable-tap-tests enable TAP tests (requires Perl and IPC::Run) > > because in practice it'll be much more likely that people will be missing > IPC::Run than that they'll be missing Perl altogether. Done. > Also, shouldn't we have it fail rather than just skipping tests if > IPC::Run is missing? Done. I was holding back on that pending the discussion on IPC::Cmd, but I don't think that will happen anytime soon.
Peter Eisentraut <peter_e@gmx.net> writes:
> On 10/30/14 9:09 PM, Tom Lane wrote:
>> Looks generally reasonable, but I thought you were planning to choose a
>> different option name?
> Yeah, but I couldn't think of a better one. (Anything involving,
> "enable-perl-..." would have been confusing with regard to PL/Perl.)
Committed patch looks good, but should we also add the stanza we discussed
in Makefile.global.in concerning defining $(prove) in terms of "missing"
if we didn't find it? I think the behavior of HEAD when you ask for
--enable-tap-tests but don't have "prove" might be less than ideal.
regards, tom lane
On 11/2/14 11:36 AM, Tom Lane wrote: > Committed patch looks good, but should we also add the stanza we discussed > in Makefile.global.in concerning defining $(prove) in terms of "missing" > if we didn't find it? I think the behavior of HEAD when you ask for > --enable-tap-tests but don't have "prove" might be less than ideal. configure will now fail when "prove" is not found.
Peter Eisentraut <peter_e@gmx.net> writes:
> On 11/2/14 11:36 AM, Tom Lane wrote:
>> Committed patch looks good, but should we also add the stanza we discussed
>> in Makefile.global.in concerning defining $(prove) in terms of "missing"
>> if we didn't find it? I think the behavior of HEAD when you ask for
>> --enable-tap-tests but don't have "prove" might be less than ideal.
> configure will now fail when "prove" is not found.
If there's a commit that goes with this statement, you neglected to push it...
regards, tom lane
On 11/3/14 3:11 PM, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> On 11/2/14 11:36 AM, Tom Lane wrote: >>> Committed patch looks good, but should we also add the stanza we discussed >>> in Makefile.global.in concerning defining $(prove) in terms of "missing" >>> if we didn't find it? I think the behavior of HEAD when you ask for >>> --enable-tap-tests but don't have "prove" might be less than ideal. > >> configure will now fail when "prove" is not found. > > If there's a commit that goes with this statement, you neglected to push it... Are you not seeing this in configure.in: # # Check for test tools # if test "$enable_tap_tests" = yes; then AC_CHECK_PROGS(PROVE, prove) if test -z "$PROVE"; then AC_MSG_ERROR([prove notfound]) fi if test -z "$PERL"; then AC_MSG_ERROR([Perl not found]) fi fi
Peter Eisentraut <peter_e@gmx.net> writes:
> On 11/3/14 3:11 PM, Tom Lane wrote:
>> If there's a commit that goes with this statement, you neglected to push it...
> Are you not seeing this in configure.in:
> AC_CHECK_PROGS(PROVE, prove)
> if test -z "$PROVE"; then
> AC_MSG_ERROR([prove not found])
Oh, duh. I read your message to mean that you'd just pushed a change
for that, not that it already did it. Nevermind ...
regards, tom lane