On Wed, 7 Nov 2001, Stas Bekman wrote:
> this patch:
> - prints the reason for the skipped test
>
> I didn't want to complicate things, so I've changed the definition of what
> a condition function should return to be:
>
> if (true)
> return 1;
> else
> return the reason as a string different from 1;
please don't do that. just return 2 values if something wants to know the
reason, something like:
my $available = eval { require $module };
my $reason = $available ? "ok" : "$module is not installed";
return wantarray ? ($available, $reason) : $available;
or have it always return a list:
return ($reason, $available);
and:
my $ok = have_foo();
will get the value of available.
that way expr if have_foo; can still be used without having to check the
length of the return value.
> solution:
> - first let's integrate this patch.
> - second I suggest splitting have_module into have_module_c and
> have_module_perl, or leave have_module as is for 'mod_*.c' but do add
> have_module_perl.
>
> consider:
>
> plan ..., have_module 'constant';
>
> for constant.pm. this will falsely satisfy the requirement with what we
> have now if there is mod_constant.c and it's compiled, but constant.pm is
> not available. There is no requirement for Perl modules to start with
> uppercase letter.
only Perl pragma modules start with a lowercase letter. i don't see this
ever being a problem and would much rather continue to be able to mix Perl
and C modules in a single have_module(...) call. if you really see
the need, just add support to have_module so you can say
have_module('constant.pm') so it will only check for a Perl module.
> - third IMHO tests shouldn't care about why their requirement is not
> satisfied, thefore we shouldn't try to make them set the reason.
then why does your patch do that? for something like a module, i would
agree, but there will be cases such as this part of you patch below where
only the test will know the reason why the it is skipping.
> --- t/apache/byterange.t 2001/09/10 17:12:37 1.2
> +++ t/apache/byterange.t 2001/11/07 06:50:34
> @@ -25,7 +25,8 @@
>
> my %other_files;
>
> -plan tests => @pods + keys(%other_files), sub { $perlpod };
> +plan tests => @pods + keys(%other_files),
> + sub { $perlpod ? 1 : "dir $vars->{perlpod} doesn't exist"};
