Doug MacEachern wrote:
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);
that won't work. The condition argument must return a single value, or it'll mess up the magic we have at the end of %args Test::plan expects.
Originally I've suggested to return a ref to a hash, since it wasn't used yet. {$meets_condition => "failure reason"}.
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.
can you please explain why do you want to mix the two? What similarity the two types have?
- 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.
this is a special example where the patch doesn't demand its requirement from the core system and therefore has to handle it by itself. And this is fine, we want the non-special cases to be handled by the core system, while allowing to have special cases.
--- 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"};
I guess I don't understand you. What you have suggested earlier is to add a special variable, so tests can set it if they have a reason to fail.
First this requires changing tests to set the reason.
Second what's the point of the plan() extension magic then? If the test says:
plan %foo, [qw(mod_a mod_b perl_mod)];
How the test can set the reason if it doesn't know which of the three modules above will fail? unless it runs these before calling plan().
Currently I see two solutions:
1. per my suggestion, each have_foo returns:
{$meets_condition => "failure reason"}.2. have a special class variable:
$Test::FailureReason which can be set by any of have_foo functions on failure and allows the test writer to set it as well.
This will allow us to keep tests simple and let have_foo subs to report the reason, without taking away the flexibility of being able to set this variable from within the test.
_____________________________________________________________________ Stas Bekman JAm_pH -- Just Another mod_perl Hacker http://stason.org/ mod_perl Guide http://perl.apache.org/guide mailto:[EMAIL PROTECTED] http://ticketmaster.com http://apacheweek.com http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
