Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Allow user to override class for creating mech object #2

Merged
merged 1 commit into from Aug 31, 2012

Conversation

Projects
None yet
2 participants
Contributor

grantm commented Aug 31, 2012

The current implementation of Test::WWW:Mechanize::Dancer explicitly hard-codes the name of the class used to create the mechanize object.

I needed to override this setting to use a class which inherits from Test::WWW::Mechanize::PSGI and adds some project-specific test helper methods. This patch allows the test script to override the default class by providing a different value in the 'mech_class' option.

Owner

throughnothing commented Aug 31, 2012

Perfect, this seems pretty useful, thanks for the patch. I'll do a release tonight after I merge them both.

@throughnothing throughnothing merged commit 3221c7d into throughnothing:master Aug 31, 2012

@throughnothing throughnothing commented on the diff Aug 31, 2012

lib/Test/WWW/Mechanize/Dancer.pm
@@ -21,7 +22,7 @@ has mech => (
default => sub {
my ($self) = @_;
- my $m = Test::WWW::Mechanize::PSGI->new(
+ my $m = $self->mech_class->new(
@throughnothing

throughnothing Aug 31, 2012

Owner

Hmm, now that I'm thinking about it, do we need to require $self->mech_class first before this can work?

@grantm

grantm Aug 31, 2012

Contributor

In my case that's taken care of by the code that calls Test::WWW::Mechanize::Dancer - and in fact a require would fail for me.

I don't think it's unreasonable for the caller to be responsible for loading the necessary code but now that you mention it, this should be mentioned in the POD.

@throughnothing

throughnothing Aug 31, 2012

Owner

Hmm, so what do you think about this:

edeb72d

Maybe I should just put it back to how you had it, and update the documentation?

Also, when you say a require would fail for you, is that because its not in your path, or what? I don't think its unreasonable to require that any modules you want to use as a mech_class be in your path...

@grantm

grantm Aug 31, 2012

Contributor

I think if people want to override he mech class they'll usually be subclassing Test::WWW::Mechanize::PSGI so having that remain as a hard dependency of Test::WWW::Mechanize::Dancer doesn't seem like a bad idea to me. Simple is good :-)

I did go back and tweak the docs here but I didn't realise you'd already merged the pull request at that point:
grantm/Test-WWW-Mechanize-Dancer@4f7e0d5

@throughnothing

throughnothing Aug 31, 2012

Owner

That's fine, I'll pull them in. Thanks for the feedback and the PR's!

@throughnothing

throughnothing Aug 31, 2012

Owner

I merged in your other POD changes and just did a release. I think i Mucked up the 0.002 release somehow, so I released to 0.003 which should be good. Thanks again.

@grantm

grantm Aug 31, 2012

Contributor

Thanks for the awesomely quick turnaround! And thanks for writing the
module in the first place - I'm finally writing some useful Dancer
tests :-)

Cheers
Grant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment