[cgiapp] Re: Request for comments: Test::WWW::Mechanize::CGIApp

[Mark Stosberg <suppressed>]

George Hartzell wrote:
> I was looking for a make-myself-useful-and-learn-something project on
> the cgiapp wiki and came across a call for a Mechanize'd tester for
> cgiapp.
>
> I put this together after looking at how the catalyst equivalent
> works.  I'd appreciate any feedback folks care to throw at it.

George,

Sorry for the delayed response-- Thanks for developing this! Overall, it 
looks great.

> It has the following issues that I'd like to straighten out:
>
>   o it has a poorly named method, "instance", which can contain either
>     the name of a class (CGI::Application or
>     CGI::Application::Dispatch derived) or a CODE ref that the
>     mechanic uses to handle the request.
>
>     I thought about "handler", but that's pretty thoroughly overloaded
>     in our world.  "request_handler"???
>
>     Anyway, I'd appreciate suggestions.

I suggest "app". Each module corresponds to a web application. That's 
also consistent with a name that C::A::Dispatch uses for a similar purpose.

>   o If you use CAP::AutoRunmode, you need to let the mechanic know so
>     that it can also 'use' it, otherwise things Just Don't Work.  The
>     way things are right now it's probably a dependency.  I'll at
>     least need to eval the use statement, but I might be able to do
>     something better.  Require/import on demand doesn't work, I think
>     because AutoRunmode does its magic at compile time.

Could this be solved the autorunmode_hack() method you added?

>   o The code'll get a little more cleanup, and the docs will get
>     fluffed up a bit.

In this realm, here are a couple of detail suggestions:

- You probably just need the second line here:

 use Test::WWW::Mechanize;
 use base 'Test::WWW::Mechanize';

- Once in the docs you mention Catalyst when I think you mean C::A.

- the docs say that new() is just like WWW::Mechanize's new(), but I 
think it's more accurate to say that it's like Test:::WWW::Mechanize::new().

> It'll do the right thing with packages that inherit from
> CGI::Application (new up an instance and call ->run()) and
> CGI::Application::Dispatch (call the class method ->dispatch()).  You
> can also give it your own sub to call if you'd like to do something
> different.
>
> Without further ado:
>
>   http://shrimp.alerce.com/cgiapp/Test-WWW-Mechanize-CGIApp-0.03.tar.gz
>
> What do you think?

Please go ahead and package a version and put it on CPAN. Because it is 
generally a development tool that likely won't be used in production 
much, I think there is even less pressure to keep a stable API, if you 
want to tweak things after the fact.

However, since Catalyst paved the way here, we are already taking 
advantage of some refinements they've made, I suspect.

  Mark


---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/suppressed/
             http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: suppressed
For additional commands, e-mail: suppressed