Greetings, Tried in 5.16.3 and 5.24.0. One of our debugging steps was to take advantage of perlbrew and try it in a different version of perl. Same result.
Interesting idea that this could be a perl bug. I don't know if I'd call this a bug though, as much as perl doing *exactly* what you asked it to without knowing what you were asking. Best Regards, Robert Stone On Fri, Dec 16, 2016 at 4:40 PM, Julian Brown via Houston <[email protected]> wrote: > 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/ >
_______________________________________________ Houston mailing list [email protected] http://mail.pm.org/mailman/listinfo/houston Website: http://houston.pm.org/
