First of I love Test::MockModule, I am wondering if you run this on different versions of Perl if your results would be different and possibly a bug in Perl for that version?
Julian On Fri, Dec 16, 2016 at 12:43 PM, Robert Stone via Houston <[email protected]> wrote: > Greetings, > > > tl;dr - refcount unexpectedly incremented, caused DESTROY method on object > to never get called. Used a workaround to address but perhaps there is a > cleaner way? > > > Jocelyn and I just spent about 3 hours tracking down why a DESTROY wasn't > being called, a whole bunch of refcounts later we found the problem. I'm > only fairly confident in my analysis below and would love it for any of you > to chime in and say "Yep! Sounds about right" or "No way, what actually > happened was..." Should be good times for all of us. > > For those of you who aren't fully versed on the magic that is > Test::MockModule, it's the greatest thing since the circumflexion > operator. It allows you to mock a method of an object and then when the > variable that you assigned Test::MockModule->new to goes out of scope then > the mock is removed. In general, very handy and it works perfectly. > > Here is the subroutine I was using to define and install my mock: > > sub restrict_search_space { > my ( $user_ids ) = pos_validated_list( \@_, { isa => 'ArrayRef[Int]' > } ); > > my $mocked_user = Test::MockModule->new( > 'FreeChat::API::Model::User' ); > $mocked_user->mock( > 'search', > sub { > my $self = shift; > my $condition = shift; > my $attributes = shift // {}; > > $condition->{'me.id'} = { 'in' => $user_ids }; > return $mocked_user->original( 'search' )->( $self, > $condition, $attributes ); > } ); > > return $mocked_user; > } > > Seems easy enough right? I create an instance of Test::MockModule that > mocks FreeChat::API::Model::User. Then I mock the search method so that I > can restrict the search space by adding an additional condition to the > WHERE clause that is ultimately passed down to DBIx's search method. > Because what I really want here is an AROUND instead of a straight up mock > I end up having to call $mocked_user->original('search') to get the > original unmocked method, thereby making this into a sort of AROUND. > > This works no problem until you try to do something like this (actual > tests! Names were not changed to protect the guilty): > > subtest 'Gender' => sub { > my ( $mech, $user ) = get_mech_and_user(); > > my $male_user = create_user( gender => 'Male' ); > my $female_user = create_user( gender => 'Female' ); > > my $mocked_user = restrict_search_space( [ $male_user->id, > $female_user->id ] ); > > $mech->get_ok( '/users?gender=Male' ); > > note( 'Raw JSON Response: ' . $mech->content ); > my $json = from_json( $mech->content ); > > ok( ( grep { $_->{id} == $male_user->id } @{$json} ), 'Matching > user correctly identified' ); > ok( !( grep { $_->{id} == $female_user->id } @{$json} ), > 'Unmatching user correctly not identified' ); > }; > > subtest 'Status' => sub { > my ( $mech, $user ) = get_mech_and_user(); > > my $active_user = create_user( status => 'Active' ); > my $offline_user = create_user( status => 'Offline' ); > > my $mocked_user = restrict_search_space( [ $active_user->id, > $offline_user->id ] ); > > $mech->get_ok( '/users?status=Active' ); > > note( 'Raw JSON Response: ' . $mech->content ); > my $json = from_json( $mech->content ); > > cmp_ok( $mech->status, '==', 200, 'Correct status' ); > > ok( ( grep { $_->{id} == $active_user->id } @{$json} ), > 'Matching user correctly identified' ); > ok( !( grep { $_->{id} == $offline_user->id } @{$json} ), > 'Unmatching user correctly not identified' ); > }; > > The first subtest passes no problem, the second one though does not. Each > one does pass in isolation, but run together they fail. It fails because > the instance of Test::MockModule never goes out of scope (the object > pointed to by $mocked_user has a refcount of 2, not of 1) and therefore the > mock never gets uninstalled. CURSES! > > See the issue? > > my $mocked_user = Test::MockModule->new( > 'FreeChat::API::Model::User' ); > $mocked_user->mock( > 'search', > sub { > my $self = shift; > my $condition = shift; > my $attributes = shift // {}; > > $condition->{'me.id'} = { 'in' => $user_ids }; > *return $mocked_user->original( 'search' )->( $self, > $condition, $attributes );* > } ); > > It seems that because I called $mocked_user here and have the entire thing > wrapped in a sub, I'm creating a closure causing the refcount on > $mocked_user to be incremented. It seems that one of the references comes > from restrict_search_space's $mocked_user and the other comes from the > subtest's $mocked_user. When I make the below modification: > > sub restrict_search_space { > my ( $user_ids ) = pos_validated_list( \@_, { isa => 'ArrayRef[Int]' > } ); > > my $mocked_user = Test::MockModule->new( > 'FreeChat::API::Model::User' ); > *my $original_method;* > $mocked_user->mock( > 'search', > sub { > my $self = shift; > my $condition = shift; > my $attributes = shift // {}; > > $condition->{'me.id'} = { 'in' => $user_ids }; > return *$original_method*->( $self, $condition, $attributes > ); > } ); > > *$original_method = $mocked_user->original( 'search' );* > > return $mocked_user; > } > > Everything worked as expected, at the end of the subtest the $mocked_user > goes out of scope and the method gets unmocked. > > The real question, while this works it seems awfully hacky and ugly. Is > there a better way to do this? Should I have used weaken somewhere? > > Just thought you all would enjoy commiserating with us. Hope everyone is > having a safe and happy holiday. > > Best Regards, > Robert Stone > > _______________________________________________ > Houston mailing list > [email protected] > http://mail.pm.org/mailman/listinfo/houston > Website: http://houston.pm.org/ >
_______________________________________________ Houston mailing list [email protected] http://mail.pm.org/mailman/listinfo/houston Website: http://houston.pm.org/
