httpd-test-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <>
Subject Re: [PATCH] new need functions
Date Mon, 02 Aug 2004 17:00:06 GMT
Geoffrey Young wrote:
> hi all
> ok, attached is a patch that implements the new need variants and alters the
> old have variants.  basically, all I did was a global rename of have to
> need, then implement the have routines in terms of need with the AutoLoader.
>  I thought this made a bit more sense than redefining all of those
> subroutines again when the only difference was simply not populating
> @SkipReasons.
> the mod_perl test suite continues to run, as does the perl-framework, so
> nothing (that I can see at least) should be broken.
> anyway, comments welcome, as well as documentation - I couldn't decide where
> to add the have stuff so I just didn't do anything.

I'd suggest to simply explain that there are have_ and need_ functions 
at the beginning of that section that explains need_* ones. And one 
should use need_* inside plan(), because of the skip messages. Otherwise 
use have_*.

Please don't commit w/o the docs. Once it's committed, the docs are most 
likely will be forgotten.

You don't want to remember to update two places when you add new 
functions. So there should be a need* functions array and when you 
populate the @EXPORT list you generate the have* list. Also there should 
be no have() function, but only need(). The rest map need_* => have_*.

Next you can use that have* list to generate the actual functions on the 
fly, similar to the generation of the subs similar to log levels subs in 
Apache::TestTrace. So you don't really need AUTOLOAD. It'll take the 
same amount of code w/o AUTOLOAD.

> +sub AUTOLOAD {
> +
> +    use vars qw($AUTOLOAD @SkipReasons);

haven't you declared the two at the top of already?

> +    (my $method) = $AUTOLOAD =~ m/.*::(.*)/;
> +
> +    return unless $method =~ m/^have/;
> +
> +    $method =~ s/^have/need/;
> +
> +    if (my $cv = Apache::Test->can($method)) {
> +        my @skip = @SkipReasons;
> +        my $rc = $cv->(@_);
> +        @SkipReasons = @skip;
> +        return $rc;

why not just:

      local @SkipReasons;
      return $cv->(@_);

> +    }
> +    else {
> +        die "could not find function $AUTOLOAD";
> +    } 
> +}
> +

Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker     mod_perl Guide --->

View raw message