Skip to content

Instantly share code, notes, and snippets.

@shanna
Created May 21, 2012 07:23
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save shanna/2760932 to your computer and use it in GitHub Desktop.
Save shanna/2760932 to your computer and use it in GitHub Desktop.
Simple example of why mocks are stupid.
# First iteration of Ad
class Ad
attr_accessor :message
def view
"For sale #{message} call #{owner.name}"
end
end # Ad
# Second iteration of Ad#view now notifies the owner his Ad was viewed.
# Ad#view still returns the same output as before but our tests will fall over because the internal implementation of Ad#view changed and the mock is incomplete.
class Ad
attr_accessor :message
def view
owner.notify "Your Ad '#{message}' was viewed."
"For sale #{message} call #{owner.name}"
end
end # Ad
require 'rspec'
# The tests.
describe Ad do
subject{ Ad.new }
it 'must show the full make and model' do
subject.stub_chain(:owner, :name){ 'Apple Arthurton' }
subject.stub(:message){ 'KTM 500 EXC' }
subject.view.should == 'For sale KTM 500 EXC call Apple Arthurton'
end
end
# A dependency of the class we'll be testing.
class Owner
attr_reader :name
def initialize name
@name = name
end
def notify text
puts text
end
end
@deepfryed
Copy link

tl;dr anyone who merges tests to master branch with mocks or stubs should be hunted down with a colt 45

  1. feature branch - mock & stub all you want when the interface design is all up in the air
  2. merge to master - once you've designed / defined your api, discard mocks & stubs with a vengeance.
  3. code on master should not have tests with mock / stubs.
  4. the only thing you should be ever mocking possibly is expensive calls to external apis with known / deterministic responses.
  5. even those you can do away with something like http://rubygems.org/gems/vcr without having to do it manually.

@perryn
Copy link

perryn commented May 21, 2012

Can't help myself. If you posted this to elicit thoughts then read on, if you are determined to give up on mocking as an all-round bad idea, then go ahead and do whatever you want in your own code shrug

@shanna

  1. as_null_object will help you protect yourself against unexpected (and irrelevant) method calls https://www.relishapp.com/rspec/rspec-mocks/v/2-10/docs/method-stubs/as-null-object

  2. Having said that, knowing to apply things like that requires forethought so you may indeed find that sometimes you change something and you have to update the tests for that thing.

It is not ideal, but I much prefer it to finding that I change something and I have to update the tests for something else.

For example, if I changed Owner.notify to send an SMS, and I was using a real Owner object to test Ad then I would have to change the setup/fixture/factory for that test, even though I didn't change Ad at all.

One of the point of mocks is to isolate tests from changes in dependencies.

  1. Another point of mocks is to make a highly coupled design painful (because setting up the mocks is painful). If you are finding yourself doing a lot of painful stub chaining, it is likely you are violating the Law of Demeter pretty badly somewhere.

@deepfryed

Dude you have it totally frickin backwards.

If there is a time that having mocks and stubs makes things painful it is precisely when your interfaces are up in the air and you have to keep updating the mocks every time you change the interface between classes.

The whole point of mocks/stubs is to de-couple your tests so that you can change the implementation of methods on a class in the future without having tests for different classes break all over the place. This is useful in the future when you need to implement new features on your code base.

If you are going to remove mocks and stubs on mainline, then you are wearing the pain of updating mocks while your interfaces are fluid and then not getting the benefit of isolation when you want to change implementation. Under these circumstances I reckon you'd be better of not unit testing at all and just relying on functional tests.

@deepfryed
Copy link

hah! atleast got you to comment a bit more than calling my ideas bad :)

you make some interesting points that i would agree with. mocks do make highly coupled design less painful but that's symptomatic treatment ain't it ?

i said mocking and more specifically stubbing is good when idea's developing because i don't care much about the implementation of a class i'm gonna talk to later, all i care about is the message and response initially.

i said mocking and stubbing is bad once the interface is flushed out because if you did write code properly (loose coupling & strong cohesion) most of the methods you need to unit test will form the bulk of the class, rest you can test in integration tests. the point being, don't need to achieve 100% coverage in unit tests by stubbing out some method in another class / object.

changing mocks and mocked class at same time also makes me throw up a bit.

bottom line tho' - the more tests you write, the more you have to maintain and less code you're writing. i can refer you to @guyking for some sage advice, he's a zen master in this area :)

@shanna
Copy link
Author

shanna commented May 21, 2012

@perryn I'm not sure I'll be able to express this as eloquently as I'd like, I'm short on book learnin' and will probably mix up CS terms at some point. I'm willing to be convinced even though all I've ever seen of mocks is questionable code and enthusiastic articles that conveniently avoid some of what I think are the most obvious questions (I'll list them below).

One of the earliest things someone told me about testing was something like "test the output of a function by the arguments passed and treat the function as a black box". After I read 'The Damians' OO Perl book back around 2000 I think I adapted that original statement to include an objects state as well as the arguments passed. Fixtures or factories therefore seem like a natural OO testing fit given nothing is assumed about the implementation of a method only given the same arguments and state the output should always the same.

On the other hand it seems to me all mocks, every single one, must have an uncomfortable level of understanding about how a method achieves its output given the arguments and state unless you were to mock every interface, argument and state the method being tested has access to. The idea that tests have any knowledge at all of the inner workings of an object seems to be at odds with the core idea that OO encapsulates the complexity.

Finally my questions:

  1. Would you ignore access control to stub protected or private methods if a method you were testing made calls to them?
  2. Even more pervasive what about instance variables which have no reader?
  3. Would you mock classes written by a third party putting blind faith in the documentation about possible states/output or would you dig through the code?
  4. Finally how do you know your mock object matches the real interface and isn't different in some subtle way without testing the state it portrays is even possible given the real class/object? If a libraries API changed a fixture or factory would fail trying to build the object or at the very least your tests would have the chance to fail given they are using real objects. A mock on the other hand would carry on as if the old interface existed, you'd never know anything was wrong until the errors start rolling in from deployed code?

Cheers,
Shane.

@dhirajbhandari
Copy link

First let me start with the example itself,
why is the owner.notify being called without it affecting the behaviour of the caller? It must be for the side-effect (i.e. it does some state change). So you have a query method that does some side-effect that is non-obvious to the user of view method. So, I would actually want my test to FAIL, when I do something like that.

Now if that owner.notify was replace with something: (0..10).each { | i | i+= 1 } (ie. something that is internal to the class that does not change my contract of the view method, I would not want my test to fail. I guess it wont.

Now the hairy case, if the owner.name was replace with owner.first_name + ' ' + owner.last_name then in effect I havent changed the contract of view method; simply changed how it does things, I dont want my test to fail (at least not the one that says 'it should return blah')
So, what would I fix it?
I have couple of options (depending on my intent)
a) When I dont care when and how the collaborator (owner) is used, I will just stub out the minimum required just so my test works
b) When I do care when and how my collaborator is invoked, I write it as a separate test that explicitly checking that my collaborator is called with the right parameters (for the ones that i care) and even the order if it is important that it called in a certain order)

So, do I like mocks/stubs?
It depends on the surface-area of the api that I am trying to mock. For example, I dont like mocking active-record calls in a typical
rails code because I might change the way I invoke active-record, and active-record api has huge surface area (I can find the same record using 20 different flavours of api call), without changing my contract.

Another case is when I have a complex algorithm I want to test (i.e. I want to test my code with a large number of inputs)
I will separate the collaborator interaction with a simpler interface so that a simple stub will suffice
and implement a functional-style method that encapsulate the complexity of the algorithm, so that I can test with a large dataset of inputs(parameters) and expected outputs(return values).

So bottom line, think about your intent when writing test and think about testability when designing
Peace.

@shanna
Copy link
Author

shanna commented May 23, 2012

@dhirajbhandari hey mate,

I think you've got a bit side tracked by what my example does rather than the problem I'm showing. Here is another example:

require 'rspec'

class Owner
  attr_accessor :name, :years_old

  def describe
    "#{name} is #{years_old} years old."
  end
end

describe Owner do
  subject{ Owner.new }

  it 'must describe the owner' do
    subject.stub(:name){ 'Apple Arthurton' }
    subject.stub(:years_old) { 65 }

    # This part of the test is fine.
    subject.describe.should == 'Apple Arthurton is 65 years old.'
  end
end

The only reason you or I know which methods to stub is because we've read the source and know what Owner#describe calls. Now the question becomes how far do we take it, what if there is no public accessors for name and years_old and I'd used instance variables and constructor arguments or what if the accessors are private?

My point is your tests need an intimate knowledge of implementation details about classes and methods that might not even be your own in order to mock them. Fine then you might say, my tests document not only the method but implementation details but you have to remember not everything you are mocking is your code, it may be a third party library. On the other hand a fixture or factory doesn't need to know a thing, I set up objects in different states and test output, I don't need to know how you got from A to B only that you did and will continue to in the future.

I am trying to understand here, it's just nobody has ever given me good justification or significant benefits for the deliberate duplication of implementation details which seems at odds with the whole idea of OO encapsulation. The single best reason I've heard so far is Rails 3 is slow so it's better if your tests avoid it as much as possible.

@dhirajbhandari
Copy link

Hey @shanna,
The point I am trying to make is that you should 'mock' the collaborators if they being called in an certain way is important to you.
Remember that ruby in O-O, not a purely functional language like haskell where the parameters and return values themselves fully describe the contract (because you cannot have side-effects without denoting it in the type).
For example, if u have a command method, you want to test that it does what is supposed to do by calling the collaborator in a certain way, not what it returns. If fact, what it returns is the most trivial aspect of such method.

But, I take you point that even when I dont care about how my collaborators are called, I still need to stub it. But whats the alternative?
Calling real object is even worse because it might behave differently or it may even be wrong or not implemented yet. I dont care about that when I am testing my method that calls that un-implemented collaborator. So, for me the question is, what is the cheapest (ie. faster to run, fast to write, fast to maintain) way to achieve it. And that depends on the context as I described in my earlier example.

For me, a well-written test (with or without the use of mocks) should have the following attributes:

  1. my test should fail if I break the contract.
  2. my test should not fail if i just change the how but I full-fill the contract.
  3. orthogonality: only 1 test should fail when I break 1 thing. eg: If I change the way I call the collaborator is called (assuming I care about the collaborator being called at all), my test that says it should call the collaborator like this should fail. And not the test that says, it should return this

Hope it makes bit more sense.

@perryn
Copy link

perryn commented Jun 3, 2012

Hi @shanna,

Finally got some time to respond to this :)

Firstly, I think we are largely in violent agreement. I think you are entirely right to have the misgivings that you do.(The ironic thing is that some of the stuff we agree on, are the very reasons I do use mocks!)

Before I answer your questions, I want to set out some ideas about the way I think about this stuff. I think maybe a lot of the confusion around mocking might come from people focusing on how, but not why...

why do I mock?

I mock to provide isolation for a particular 'thing' that I am trying to test. (often this 'thing' is a class, so I will assume that from now on)

The reason I want to test classes in isolation is so that I can 'divide and conquer' to solve a problem.If I have a problem I want to solve, I break it up into a series of smaller problems, and make each the responsibility of a class.

I then want to be able to test that each of those classes solves their smaller problem correctly using unit tests. Then, I can use integration tests to check that together they solve the larger problem.

A log time ago, when I used to wire IC chips together on a bread board, and something didn't work I was pretty sure it was my wiring rather than faulty chips because the chips were thoroughly tested before leaving the factory. Basically, I'm trying to get the same level of confidence for individual classes so when something doesn't work it is easier to track down.

When I am unit testing I want to be able to focus on checking a class solves their smaller problem correctly without having to worry about the other bits of the larger problem. Hence I provide mocks for the classes that solve the other bits. This means that the unit test for one class should not break or have to be modified if the implementation of another class changes.

For example, in your first code example I think of the 'notify the owner an ad was viewed' as being broken up into

  • construct a message about the ad being viewed
  • deliver the message

The message construction is the responsibility of the Ad class, but how that message is actually delivered ( puts, email, text message) is not its concern. I therefore think that my unit test for the Ad class should not break or need to be changed if I change how the message is delivered.

what is inside the box?

I totally agree that we want to treat a class as a black box. In my test I want to look at what is going into the box, and what is coming out, but I want to be able to ignore what happens inside the box to produce that output from that input.

I think where we might be getting our wires crossed is that I have a different view about what things constitute output and input.
I think we would agree that output/input includes

  • things passed to methods (input)
  • things returned from methods (output)

but I would also include

  • things passed to methods called on collaborators by the class I am testing (output)
  • things returned to the class from those methods (input)

So once again with your first example - I consider the message passed to the notify method to be output.

I totally agree however, that the internal state of the object is not input or output and my test should not have knowledge of it. If I think about it though, that state must have been constructed from input at some stage, so my test can have knowledge on how to provide that input.

the questions

So with all that rambling in mind, let me answer your questions

  1. Would you ignore access control to stub protected or private methods if a method you were testing made calls to them?

God no. Private methods are 'inside the box' they are none of the test's business. I am testing the class, not individual methods - if the class is so complicated that I want to mock private methods then I would look at breaking the problem up into smaller problems, and the class into several classes.

The caveat to this is when I am forced to do it by icky design decisions in frameworks like the template pattern (Rails I am looking at you). Hence I will sometimes mock 'private' methods provided by a framework, but I feel dirty every time.

  1. Even more pervasive what about instance variables which have no reader?

God no. They must have got set somehow, so I would use that mechanism.

  1. Would you mock classes written by a third party putting blind faith in the documentation about possible states/output or would you dig through the code?

To be sure we are talking about the same thing, the rest of my answer here assumes you are talking about unit testing of my code that uses third party classes ( not testing the third party classes themselves)

Remember from before that I am trying to unit test that my code provides the third party class with the correct input and uses the third party api in the way I think it should, but not that calling it in that way actually works. That is an integration test's job.

So for example If I want to send a message to someone and my class uses an email sending api, then I want to unit test that I am generating the content of the email correctly, not that an email gets sent ( I'd do that with an integration test).

In fact, in this case I would usually insulate my class from the third party api. For example I might introduce a Message class (with a send method that calls the third party api) and use that in my class. To unit test my class I would mock Message and check my class creates it with the right content ( because my class is responsible for the smaller problem of generating the message content)

If all Message does is call the third party api I would probably not unit test it because that would prove little of value, now that I have broken up the message generation behaviour from the sending behaviour.

  1. Finally how do you know your mock object matches the real interface and isn't different in some subtle way without testing the state it portrays is even possible given the real class/object? If a libraries API changed a fixture or factory would fail trying to build the object or at the very least your tests would have the chance to fail given they are using real objects. A mock on the other hand would carry on as if the old interface existed, you'd never know anything was wrong until the errors start rolling in from deployed code?

I think I probably dealt with this in my previous question, but
a) my unit tests are not trying to prove I'm calling the api correctly, but that assuming I'm calling it correctly that I'm passing it the input I want
b) I would have integration tests to catch that the API has changed

OK I think I'm rambling now - this would be so much easier face to face with a few beers and a whiteboard, but hopefully it gave you some insight into how I think about this stuff?

Pez

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