<div dir="ltr">Greetings,<div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>Best Regards,</div><div>Robert Stone</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 16, 2016 at 4:40 PM, Julian Brown via Houston <span dir="ltr"><<a href="mailto:houston@pm.org" target="_blank">houston@pm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">First of I love Test::MockModule,<div><br></div><div>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?</div><div><br></div><div>Julian</div></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Fri, Dec 16, 2016 at 12:43 PM, Robert Stone via Houston <span dir="ltr"><<a href="mailto:houston@pm.org" target="_blank">houston@pm.org</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><div dir="ltr"><span style="font-size:12.8px">Greetings,</span><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">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?</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">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.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">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.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Here is the subroutine I was using to define and install my mock:</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><div><font face="monospace, monospace"> sub restrict_search_space {</font></div><div><font face="monospace, monospace"> my ( $user_ids ) = pos_validated_list( \@_, { isa => 'ArrayRef[Int]' } );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> my $mocked_user = Test::MockModule->new( 'FreeChat::API::Model::User' );</font></div><div><font face="monospace, monospace"> $mocked_user->mock(</font></div><div><font face="monospace, monospace"> 'search',</font></div><div><font face="monospace, monospace"> sub {</font></div><div><font face="monospace, monospace"> my $self = shift;</font></div><div><font face="monospace, monospace"> my $condition = shift;</font></div><div><font face="monospace, monospace"> my $attributes = shift // {};</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> $condition->{'<a href="http://me.id/" target="_blank">me.id</a>'} = { 'in' => $user_ids };</font></div><div><font face="monospace, monospace"> return $mocked_user->original( 'search' )->( $self, $condition, $attributes );</font></div><div><font face="monospace, monospace"> } );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> return $mocked_user;</font></div><div><font face="monospace, monospace"> }</font></div></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">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<wbr>') to get the original unmocked method, thereby making this into a sort of AROUND.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">This works no problem until you try to do something like this (actual tests! Names were not changed to protect the guilty):</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><div><font face="monospace, monospace"> subtest 'Gender' => sub {</font></div><div><font face="monospace, monospace"> my ( $mech, $user ) = get_mech_and_user();</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> my $male_user = create_user( gender => 'Male' );</font></div><div><font face="monospace, monospace"> my $female_user = create_user( gender => 'Female' );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> my $mocked_user = restrict_search_space( [ $male_user->id, $female_user->id ] );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> $mech->get_ok( '/users?gender=Male' );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> note( 'Raw JSON Response: ' . $mech->content );</font></div><div><font face="monospace, monospace"> my $json = from_json( $mech->content );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> ok( ( grep { $_->{id} == $male_user->id } @{$json} ), 'Matching user correctly identified' );</font></div><div><font face="monospace, monospace"> ok( !( grep { $_->{id} == $female_user->id } @{$json} ), 'Unmatching user correctly not identified' );</font></div><div><font face="monospace, monospace"> };</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> subtest 'Status' => sub {</font></div><div><font face="monospace, monospace"> my ( $mech, $user ) = get_mech_and_user();</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> my $active_user = create_user( status => 'Active' );</font></div><div><font face="monospace, monospace"> my $offline_user = create_user( status => 'Offline' );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> my $mocked_user = restrict_search_space( [ $active_user->id, $offline_user->id ] );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> $mech->get_ok( '/users?status=Active' );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> note( 'Raw JSON Response: ' . $mech->content );</font></div><div><font face="monospace, monospace"> my $json = from_json( $mech->content );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> cmp_ok( $mech->status, '==', 200, 'Correct status' );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> ok( ( grep { $_->{id} == $active_user->id } @{$json} ), 'Matching user correctly identified' );</font></div><div><font face="monospace, monospace"> ok( !( grep { $_->{id} == $offline_user->id } @{$json} ), 'Unmatching user correctly not identified' );</font></div><div><font face="monospace, monospace"> };</font></div></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">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!</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">See the issue?</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><div><font face="monospace, monospace"> my $mocked_user = Test::MockModule->new( 'FreeChat::API::Model::User' );</font></div><div><font face="monospace, monospace"> $mocked_user->mock(</font></div><div><font face="monospace, monospace"> 'search',</font></div><div><font face="monospace, monospace"> sub {</font></div><div><font face="monospace, monospace"> my $self = shift;</font></div><div><font face="monospace, monospace"> my $condition = shift;</font></div><div><font face="monospace, monospace"> my $attributes = shift // {};</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> $condition->{'<a href="http://me.id/" target="_blank">me.id</a>'} = { 'in' => $user_ids };</font></div><div><font face="monospace, monospace"> <b>return $mocked_user->original( 'search' )->( $self, $condition, $attributes );</b></font></div><div><font face="monospace, monospace"> } );</font></div></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">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:</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><div><font face="monospace, monospace"> sub restrict_search_space {</font></div><div><font face="monospace, monospace"> my ( $user_ids ) = pos_validated_list( \@_, { isa => 'ArrayRef[Int]' } );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> my $mocked_user = Test::MockModule->new( 'FreeChat::API::Model::User' );</font></div><div><font face="monospace, monospace"> <b>my $original_method;</b></font></div><div><font face="monospace, monospace"> $mocked_user->mock(</font></div><div><font face="monospace, monospace"> 'search',</font></div><div><font face="monospace, monospace"> sub {</font></div><div><font face="monospace, monospace"> my $self = shift;</font></div><div><font face="monospace, monospace"> my $condition = shift;</font></div><div><font face="monospace, monospace"> my $attributes = shift // {};</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> $condition->{'<a href="http://me.id/" target="_blank">me.id</a>'} = { 'in' => $user_ids };</font></div><div><font face="monospace, monospace"> return <b>$original_method</b>->( $self, $condition, $attributes );</font></div><div><font face="monospace, monospace"> } );</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> <b>$original_method = $mocked_user->original( 'search' );</b></font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> return $mocked_user;</font></div><div><font face="monospace, monospace"> }</font></div></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Everything worked as expected, at the end of the subtest the $mocked_user goes out of scope and the method gets unmocked.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><span style="background-color:rgb(255,255,0)">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?</span></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Just thought you all would enjoy commiserating with us. Hope everyone is having a safe and happy holiday.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Best Regards,</div><div style="font-size:12.8px">Robert Stone</div></div>
<br></div></div>______________________________<wbr>_________________<br>
Houston mailing list<br>
<a href="mailto:Houston@pm.org" target="_blank">Houston@pm.org</a><br>
<a href="http://mail.pm.org/mailman/listinfo/houston" rel="noreferrer" target="_blank">http://mail.pm.org/mailman/lis<wbr>tinfo/houston</a><br>
Website: <a href="http://houston.pm.org/" rel="noreferrer" target="_blank">http://houston.pm.org/</a><br></blockquote></div><br></div>
<br>______________________________<wbr>_________________<br>
Houston mailing list<br>
<a href="mailto:Houston@pm.org">Houston@pm.org</a><br>
<a href="http://mail.pm.org/mailman/listinfo/houston" rel="noreferrer" target="_blank">http://mail.pm.org/mailman/<wbr>listinfo/houston</a><br>
Website: <a href="http://houston.pm.org/" rel="noreferrer" target="_blank">http://houston.pm.org/</a><br></blockquote></div><br></div>