Are mocks in unit tests dangerous in dynamic languages?

Question

I've started relying heavily on a mocking framework in php for my unit tests.

My concern is that with a dynamic language, there is no way of enforcing a return type. When mocking, you have to ensure that the return type of your mocked method is correct.

Take for example the following interface

interface MyInterface {

  /**
   * @return SomeObject
   */
   public function myMethod();

}

If we are unit testing a class which requires an instance of MyInterface, we would mock the myMethod function to return a known SomeObject.

My concern is what happens if we accidentally have our mocked method return a different type of object, e.g. SomeOtherObject, and SomeOtherObject and SomeObject are not the same duck type (don't have the same methods). We'll now have code that is relying on havingSomeOtherObject, when all implementations ofMyInterfacemyMethodwill returnSomeObject`.

For example, we could write the following production code;

$someOtherObject = $myInterfaceImp->myMethod();
$newObj = new NewObject($someOtherObject);

Hopefully the issue is obvious. We are passing what we think is SomeOtherObject into a constructor. Our tests will pass, but its very likely that in production we will get failures as myMethod() will actually be returning SomeObject. We then try passing this into the constructor, and get an error if there is a type check, or a non existent method is called.

Unless we write very detailed integration and acceptance tests using the real implementations (basically we'd have to ensure they covered all the testing we do an the unit level) we may not ever realise that we have an issue until we receive some runtime error in production.

Other than developer discipline, is there any way to mitigate the risk of incorrect mocks and implementations?

Answer 1

The answer to this question "are mocks in unit tests dangerous in dynamic languages?" is "Hell yes!"

The following article explains the dangers.

They describe a scenario where an interfaces method name changed. For example, changing a methods name from error() to errors(). If we are mocking the error() method in our tests, they will never turn red to alert us that we have a problem.

Instead, mocks should be used sparingly. Behaviours of the unit under test should be tested rather than testing implementation details, which happens as a consequence of using mocks in unit tests. Real classes should be used as dependencies, instead of an over reliance on mocks.

Answer 2

You don't have to test for the correct return type in a dynamically-typed language, unless the method is specifically designed to return a specific type (i.e. a Factory). Rely instead on other kinds of tests that are more meaningful from a program logic perspective.

To give a simple (and admittedly contrived) example, if I expected a method to return a long, but it returned an int instead, the returned value would be incorrect for any numeric result that is too large to fit in an int.

The whole point of using a dynamically-typed language is so that you don't have to think about types so much. Otherwise, why would you trade off a strongly-typed language for a dynamically-typed one with type tests, when you could have let the compiler do all of that work for you automatically?

Most dynamically-typed languages have standard practices that you can use to minimize the risks. For example, in Javascript, we use === instead of == to avoid unexpected type coercion during numeric comparisons, and use numbers scaled up to pennies in money calculations to avoid binary rounding errors (all numbers are floating point in Javascript).

_{Worth noting: Factory methods are less important in dynamically-typed languages than they are in staticly-typed ones.}

Answer 3

Before I start I will add come context to question to highlight the problem. Consider:

class SUT {
  public function tested(MyInterface $x) {
    $this->hidden($x->myMethod());
  }
  protected function hidden($x) {
    $x->aMethodOfSomeObject()
  }
}

//Somewhere in test:
$sutInstance($myInterfaceMock);

Now. Your MyInterface::MyMethod returns SomeObject and your test do its job. But when you change MyInterface::MyMethod to return SomeOtherObject then your test will still pass (false positive with is wrong)
And there is even worse: if you remove MyMethod from MyInterface then this test will still get pass.

Bad news:

You can't do much about it in your test code.
It must be done on mocking framework level.
I checked PHPUnit and there is a task for mocking non existing methods. It was created in 2010 and still not closed (so from my point of view creators of PHPUnit are not interested in creating this crutial functionality).

Good news:

You can contribute to PHPUnit or whatever you use and fix these problems. Long time ago when I wasn't overworked I created small mocking library and I tried to solve these problems ( https://github.com/farafiri/Smock ).
The way I did it was parsing PHPDoc and searching for @return annotation. Every time when mocked method (in this case MyInstace::MyMethod) was called, returned type was checked aganist type from annotation.
In your example, after change $mock->MyMethod result (SomeObject) is type tested (type from annotation is SomeOtherObject) and since SomeObject is not instance of SomeOtherObject test fails. (Example: mocked method returns string when @return integer is defined in method annotation: https://github.com/farafiri/Smock/blob/master/tests/MockTest.php line 1242)

---

1. You can try deffensive programing. Check all arguments in methods if the are correct types.
   Still, your test will say nothing, but it blow up sooner
   (maybe during developer work, testers will find it or behavoiural test will let you know if you use any) and less likely it will blow up in end user face.

2. Use type hinting whenever possible. During development it will be much easier to find methods (and their test) which you should check.

3. Improve your testing tools. This is elaboration on my prevoius idea from "Good new" part. You have to invest some time for this and only if you are using annotations for return value (and I belive this is the case). In case of PHPUnit you can replace

   $myInterfaceMock->expects($this->any())
       ->method('myMethod')
       ->will($this->returnValue($someObjectMock)); 
   //with
   $myInterfaceMock->expects($this->any())
       ->method('myMethod')
       ->will($this->myReturnValue($someObjectMock, 'MyInterface', 'MyMethod'));
   
   //and in your test class:
   
   protected function myReturnValue($value, $class, $method) {
       if (!checkMethodExists($class, $method)) {
           throw BadTestException();
       }
   
       $expectedType = getExpectedType($class, $method);
   
       if (!isTypeOf($value, $expectedType) {
           throw BadTestException();
       }
   
       return $value;
   }
   

   Using ready annotation readers implementation for 98% most common scenarios should take you about 2 days.
   Extending it for more PHPUnit methods like returnCallback or onConsecutiveCalls and so on should take 1 more day.

4. Do not rely on mocks to much. Use real object instead of mocks whenever it is possible. If you would create real $myInstanceObject whitch creates SomeOtherObject instead of SomeObject expected by tested method, test would fail.

5. I think this problem is a bit beyond unit testing domain. This kind of errors should be handled on functional or behavioural tests level.

Answer 4

I'd say that mocks are dangerous in every language.

Let's say that I call:

priceCalculator.calculate(
    qty,
    item,
    isPlusCustomer,
    isSenior); 

What if:

1. isPlusCustomer and isSenior are supposed to be in the other order?
2. qty was supposed be a weight, not a qty.
3. you needed to initialize the priceCalculator first before using it?

There are lots of ways for mocks to fail to detect problems. Some subset of these are resolved by statically typed languages. But in general the problem remains.

There are two solutions that I'm aware one.

The first is contract tests. The idea here is that if you one test that says via mocks:

expect(priceCalculator.calculate(5, item, true, false)).returns(5.00);

You need a contract test elsewhere that says:

assert(new PriceCalculator().calculate(5, item, true, false)).returns(5.00);

So that every expectation you set has a matching test to ensure it actually works the way you claim it does.

The second is not to use mocks. Instead of mocking out the PriceCalculator, just use the real PriceCalculator. Instead of worrying about mocking the PriceCalculator correctly, you can actually test against the real PriceCalculator.

Answer 5

is there any way to mitigate the risk of incorrect mocks and implementations?

In python-mock, you can specify a mock spec. A spec constraints what methods and attributes are available in the mocked object, thus you'll get an error if the system under test (SUT) try to call or access something unexpected.

My concern is what happens if we accidentally have our mocked method return a different type of object e.g. SomeOtherObject? We'll now have code that is relying on having SomeOtherObject, when all implementations of MyInterface myMethod will return SomeObject.

You shouldn't worry about actual types. Actual types should be considered implementation details; it's the duck type that you should be the more concerned about. Test that the correct methods are called with the correct arguments, you usually shouldn't need to test the actual type of the return value. Instead of asserting the actual type, assert the state that you expect for the object to be in, after the SUT.

Unless we write very detailed integration and acceptance tests (basically we'd have to ensure they covered all the testing we do an the unit level) we may not ever realise that we have an issue until we receive some runtime error in production.

Mocks are for unit tests, you should not be using mocks for integration and acceptance tests. Integration and acceptance tests should use the real objects as much as possible, and only use very minimal mock to replace external systems.that may not be fully under your control.