Mailing-List: contact test-dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: test-dev@httpd.apache.org
Date: Wed, 7 Nov 2001 10:10:24 -0800 (PST)
From: Doug MacEachern <dougm@covalent.net>
To: test-dev@httpd.apache.org
Subject: Re: [patch take2] add test skipping reasoning
In-Reply-To: <Pine.LNX.4.40.0111071500030.7124-100000@hope.stason.org>
Message-ID: <Pine.LNX.4.21.0111070954050.27309-100000@localhost>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII

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"};