Skip to content
New issue

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

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Delegate hash access on spy to the real object #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Delegate hash access on spy to the real object #22

wants to merge 3 commits into from

Conversation

showaltb
Copy link

The commit at 2bd4439 to pass the spy to method calls works great for method dispatch, but breaks direct access to the blessed object (hash reference).

For example, the following fails under 0.65:

{
    package MyClass;
    sub new { bless { foo => 'FOO' }, shift }
    sub say_foo { my $self = shift; print $self->{foo}, "\n" }
}

use Test::Mocha;
my $obj = MyClass->new;
my $spy = spy($obj);
$spy->say_foo;     # should print "FOO"

Instead of printing 'FOO', the last line prints nothing (undef), because in sub say_foo, $self->{foo} is accessing the foo element of the spy object instead of the real object.

This PR converts the spy to a tied hash (using the core Tie::ExtraHash module) so that direct hash operations are delegated to the real object.

The spy's attributes can be access through tied($spy)->[1]. This technique is applied in the __object, __calls, and __stubs methods.

As a limitation, spy() only accepts blessed HASH references. That probably covers the vast majority of use cases. Support for blessed SCALAR and ARRAY references can be added fairly easily.

@showaltb
Delegate hash access on spy to the real object
215de13 
Converts the spy to a tied hash so that direct hash operations are
delegated to the real object. Adds restriction that spy() can
only accept a blessed HASH reference.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 215de13 on showaltb:spyhash into ea1b29f on stevenl:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 215de13 on showaltb:spyhash into ea1b29f on stevenl:master.

@stevenl
Copy link
Owner

stevenl commented Feb 4, 2019 via email

@showaltb
Copy link
Author

showaltb commented Feb 5, 2019 •
edited
Loading

Hi Steven,

Here's a silly example to illustrate what I'm trying to do. Consider the following "temperature conversion" class:

  package TempConverter;

  sub new
  {
    my $class = shift;
    bless {
      factor => 1.8,
      offset => 32,
    }, $class;
  }

  sub temp_from_service
  {
    my $self = shift;
    # ... imagine a process that consults an external web service
    print "(external service called)\n";
    10;
  }

  sub converted_temp
  {
    my $self = shift;
    my $temp = $self->temp_from_service;
    $temp * $self->{factor} + $self->{offset};
  }

Now I want to write a unit test for the converted_temp method. I'd like to stub the temp_from_service method so that my test is not dependent on external services. So I'd like to something like this:

use Test::Mocha;
my $subject = TempConverter->new(foo => 'CUSTOM');
my $spy = spy($subject);
stub { $spy->temp_from_service } returns(5);
$spy->converted_temp;   # <-- expect result 41 here

Under Test::Mocha 0.64, the stub is ignored and the actual temp_from_service is called (because $self is the actual object and not the spy). The output is 50 (based on the the temp_from_service return).

Under Test::Mocha 0.65, the stub is called and $temp is set to 5. However, the following line doesn't work properly, because factor and offset are accessing the blessed spy hash and not the original object. The output is 0.

Under this PR, the test works properly and the the expected output of 41 is produced. The accesses to factor and offset are delegated to the original object.

I'm used to Ruby test libraries like mocha and rspec-mocks, where this kind of stubbing works fine. Perl complicates things because accesses to data on the blessed object does not go through normal method dispatch but instead requires the tie mechanism.

An alternate way to approach this test would be to use something like Sub::Override to stub TempConverter::temp_from_service. At that point, i don't need the Test::Mocha spy at all. But I'm keen to use Test::Mocha throughout, as it feels much more like a "modern" mocking/stubbing system.

I hope that makes sense. Thanks for your consideration.

@stevenl
Copy link
Owner

stevenl commented Feb 7, 2019 via email

@showaltb
Copy link
Author

Hi Steven,

I just added a couple of minor fixes to this PR.

I wonder if you'd had a chance to think about this direction any more? This pattern is extremely useful to me, but perhaps I'm using an unconventional (or even ill-conceived) testing strategy?

If you're not interested in extending the current spy like this, would you be open to adding a separate kind of spy that would have this extended behavior?

use Test::Mocha;
my $obj = MyClass->new;
my $spy = spy($obj);           # create an 0.65-style spy
my $ext_spy = ext_spy($obj);   # create an extended spy that delegates

Or, failing that, would you be open to a few tweaks to the internals that would allow this extended spy to be implemented in a subclass?

use Test::Mocha::ExtSpy;      # extends behavior of spy
my $obj = MyClass->new;
my $spy = spy($obj);          # create an extended spy that delegates

Just some ideas...

@stevenl
Copy link
Owner

stevenl commented May 7, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@showaltb @coveralls @stevenl