httpd-test-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject Re: Fix: Apache::TestRequest::redirect_ok
Date Wed, 20 Oct 2004 01:26:40 GMT
David Wheeler wrote:
> On Oct 19, 2004, at 2:48 PM, Stas Bekman wrote:
> 
>> In which case, can you please review Boris' patch and commit it if you 
>> think it's good? As I haven't coded and haven't used much this 
>> feature, I'd rather let somebody who is more familiar with it do the 
>> decision. If it breaks something, we can always fix that later. And of 
>> course when changing something it's a good idea to add a new test 
>> which exercises that fix. Thanks.
> 
> 
> I hate this code.
> 
> I think that the original idea was that, if LWP was installed, then 
> redirects from POST were supported. Otherwise, they were not, as 
> Apache::TestRequest didn't have the code to handle it. (Why? I have no 
> idea.)
> 
> If that's the case, then Boris' code makes decent sense, beucause the 
> call to can('SUPER::redirect_ok') will return a code reference if LWP is 
> installed and TestRequest inherits from it. Otherwise, it decides for 
> itself. So to keep things consistent, I would actually do this:
> 
> --- TestRequest.pm.~1.100.~    Tue Oct 19 18:08:08 2004
> +++ TestRequest.pm    Tue Oct 19 18:09:24 2004
> @@ -199,8 +199,9 @@
>  $RedirectOK = 1;
> 
>  sub redirect_ok {
> -    my($self, $request) = @_;
> -    return 0 if $request->method eq 'POST';
> +    my $self = shift;
> +    return $self->SUPER::redirect_ok(@_) if $have_lwp;
> +    return 0 if shift->method eq 'POST';
>      $RedirectOK;
>  }

that looks better than goto :)

> But this means that POST redirects will work if LWP is installed and 
> won't if it is not. If it's true that, without LWP, TestRequest *cannot* 
> handle redirect on POST requests, then this is how it should be.

I believe that it can't. I think TestRequest doesn't try to provide a 
replacement for lwp. It's a very basic client which should be used only 
for very basic tests. One should add need_lwp in the plan() call for more 
complicated cases.

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Mime
View raw message