Skip to content

Instantly share code, notes, and snippets.

@JeffreyWay
Last active December 16, 2015 04:49
Show Gist options
  • Save JeffreyWay/5380399 to your computer and use it in GitHub Desktop.
Save JeffreyWay/5380399 to your computer and use it in GitHub Desktop.
I've been spending a lot of time lately figuring out how to make testing as simple as possible. Mocking is a sore point for lots of folks. What do you think of this, for more natural mocking?
<?php
public function testIndex()
{
// Mock the Dog class
// Expect all() method to be called
// and return 'foo'
$this->dog->all()->shouldReturn('foo');
// Call /dogs (DB won't be hit)
$this->call('GET', '/dogs');
// Make sure view has $dogs var
$this->assertViewHas('dogs', 'foo');
}
public function testShow()
{
// Mock the Dog class
// Expect find() method to be called
// with argument, 1
// and return 'foo''
$this->dog->find(1)->shouldReturn('foo');
$this->call('GET', '/dogs/1');
$this->assertViewHas('dog', 'foo');
}
@JeffreyWay
Copy link
Author

Is this more readable to you guys?

Rather than doing something like $m->shouldReceive('find')->with(1)->andReturn('foo'), I thought that it might feel more natural to write your mock in the same way that you would in the production class.

$this->dog->find(1)->shouldReturn('foo');

@danharper
Copy link

I think you should definitely need to say "receives"/"receives once", for clarity. It's not clear that you're mocking the method, instead of stubbing it.

How about:

<?php
$this->dog->calls()->all()->shouldReturn(['foo', 'bar']);
$this->dog->calls()->all()->once()->shouldReturn(['foo', 'bar']);
$this->dog->calls()->find(7)->twice()->shouldReturn('baz');

calls being shouldReceive; and the expected call to (eg. find()) can receive the expected with values?

Edit: calls would be better as receives

Edit 2: and maybe shouldReturn could just be returns

<?php
$this->dog->receives()->all()->once()->returns(['foo', 'bar']);

Edit 3: But idk what you're aiming for with this? An entire extension of mockery for a cleaner syntax, or just a small bit of sugar built into the class?

@JeffreyWay
Copy link
Author

At that point, though, it's not much different than the current way of mocking. The goal was to make it feel as production-like as possible.

@danharper
Copy link

Very true (see my third edit above).

@JeffreyWay
Copy link
Author

Ahh - this is just a bit of sugar. It's only a few methods.

@danharper
Copy link

In that case, yeah, I like the premise of it!

@alexrussell
Copy link

I suppose it's personal preference, but doing it the 'normal' PHPUnit/Mockery way reads more correctly in my opinion. This way is more concise and, as you say, allows you to 'call' the methods as if in the controller logic, but to me that's its problem. The line that's saying "x should do y and then z" is actually saying "call y on x and that should return z" in the test code, which in my opinion breaks the meaning (and thus readability) of the code, if you see what I mean?

It almost seems as if that line is doing some test preprocessing (for example, setting something up) rather than actually declaring the test parameters, I guess is what I'm saying.

@JeffreyWay
Copy link
Author

I get what you're saying (that's the reason behind my Tweet for second opinions.)

The issue is that the traditional way requires a lot of boilerplate - so much to the point that it scares people away. For example, this code:

$this->dog->all()->shouldReturn('foo');

...behind the scenes, what is happening is something along the lines of:

$mock = Mockery::mock('Dog');
$mock->shouldReceive('all')->once()->andReturn('foo');
$this->app->instance('Dog', $mock); # L4 update IoC container instance

To my eyes, the former is much more readable and intuitive. It doesn't require as much visual "decoding" every time I read it. "When /dogs is called, $dog->all() should be triggered, and return foo."

@alexrussell
Copy link

Grr Github bug just lost my reply. Vague idea was along these lines:

I think as a convenience function around the boilerplate then it does it quite well, but I still think it looks possibly misleading. It's hard to tell at a glace whether you're calling all on an instance variable you've previously set up (maybe in the constructor) called dog or whether you're doing this mock set-up.

As I said before I think it really comes down to personal preference. If you're the only person working on your code then go for it. If you need people to be able to understand your code who may never have seen this sugar before, then you may be asking for trouble.

Edit: typo

@JeffreyWay
Copy link
Author

I decided to extend it just a bit to allow people to write the mocks however they wish.

All of the same will achieve the same result:

$this->post->find(1)->shouldReturn('foo');

$this->post->shouldReceive('find')->with(1)->andReturn('foo');

$this->mocked('post')->shouldReceive('find')->with(1)->andReturn('foo');

$this->mock('post')->andExpect('find')->with(1)->toReturn('foo');

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