An unexpected call to one of the method of a mock doesn't always fails a test

[dzamlo]

An unexpected call to one of the method of a mock raise an org.scalatest.exceptions.TestFailedException. But if this exception is somehow suppressed, the failure is silently ignored.

ScalaMock Version (e.g. 3.5.0)

4.1.0

Scala Version (e.g. 2.12)

2.12.4

Runtime (JVM or JS)

JVM

Please describe the expected behavior of the issue

The test should fail with an "Unexpected call: ..." error.

Please provide a description of what actually happens

The test doesn't fails.

Reproducible Test Case

import org.scalamock.scalatest.MockFactory
import org.scalatest.FlatSpec

import scala.util.control.NonFatal

trait Foo {
  def bar()
}

class FooBarTest extends FlatSpec with MockFactory {
  "foo" should "bar" in {
    val m = mock[Foo]

    try {
      m.bar()
    } catch {
      case NonFatal(e) => println(e)
    }
  }
}

[barkhorn]

why would this test fail if you wrap the call in a try/catch?
it does print the exception which would usually make the test fail.
Remove the try/catch and it works fine

[dzamlo]

I would expect the test to fail if any of the "Expectations" are not met. If I set the expectation that the method is called twice (with (m.bar _).expects().repeat(2)), I would expect the test to fail if I call the method 3 times. Even if the exception raised at the moment of the call is suppressed.

In this minimal example, it's easy to just remove the try/catch. But in my real use-case the mock is used in a place where I need to handle any arbitrary exception. To add to that, the call is inside a Future, inside an Akka actor, so I cannot just let the exception be raised.

My expectation is that when the expectation are checked at the end of a test, the mock is also tested for unexpected calls.

[barkhorn]

you do have the choice between mock, which has this behaviour, and stub, which uses verify after test.
If your test double is used in a path where exceptions are silently dropped, then maybe using a stub will help you out.
See http://scalamock.org/user-guide/mocking_style/ for more info

[KarelCemus]

I vote for this. MockContext records expectations and callLog but directly reports unexpected calls, which is inconsistent. Would it be an issue to also record instead of report unexpected calls (possibly optionally) and let a test fail within verifyExpectations(), when there some exist?

For example, when the test assumes 2 calls of a function but there are actually 3 calls, it silently passes while when there would be only one, it would fail.

[barkhorn]

@KarelCemus please provide a concrete example where you think an issue exists. I pointed out the relevant documentation above regarding mock / stub behaviour and you already have a choice of "expectations first" or "record then verify".

[JDolinger]

Wow, I'm having the exact same issue being reported above. I'm mocking a call that returns a Future. I want to test the behavior which is Future.failed(some exception....). Farther up the stack we do a recover on that exception type so we can turn it into a reasonable error message from a REST call (using play framework). So the exception that is getting thrown by scalamock invokes that block, and the test silently passes if there is some unexpected mock invocation... I also would vote for the "record unexpected call" and then verify behavior.

[KarelCemus]

I have a very similar scenario - use a mock within an Akka actor responsible for network communication and it is supposed to recover from all exception, which unfortunately includes the exception thrown by the mock itself, which leads me to the need of manual verification of both expected and unexpected calls at the end of a test.

However, I'd like to point to the inconsistency. While the expectations are evaluated at the end of a test, unexpected calls are reported directly during a test. I am aware of the issue that late reporting of unexpected calls may hide an original cause of a test failure and the test may fail due to a different reason but still, there should exist an option to manually report unexpected calls, therefore report them twice or switch a mechanism.

If I get it right, a mock is an object with simulated behavior and we are interested in monitoring all interactions with such an object, which includes unexpected call, while a stub does not have to be monitored at all. Therefore, swapping them does not seem semantically correct.

Implementation of the verification of unexpected calls is actually pretty simple, it is like 4 lines of code. I already did it myself, although I had to copy-paste a lot of ScalaMock's code as it is pretty closed (lots of private) and it is not easy to extend it.

trait MockFactoryRecordingUnexpectedCalls extends MockFactoryBase {

  private var unexpectedCalls = new CallLog

  override private[scalamock] def reportUnexpectedCall(call: Call): Nothing = {
    unexpectedCalls += call
    super.reportUnexpectedCall(call)
  }

  override protected def withExpectations[T](what: => T): T = {
    if (expectationContext == null) {
      // we don't reset expectations for the first test case to allow
      // defining expectations in Suite scope and writing tests in OneInstancePerTest/isolated style
      initializeExpectations()
    }

    try {
      val result = what
      reportUnexpectedCalls()
      verifyExpectations()
      result
    } finally {
      clearExpectations()
    }
  }

  private def initializeExpectations() {
    val initialHandlers = new UnorderedHandlers
    callLog = new CallLog
    unexpectedCalls = new CallLog

    expectationContext = initialHandlers
    currentExpectationContext = initialHandlers
  }

  private def clearExpectations(): Unit = {
    // to forbid setting expectations after verification is done
    callLog = null
    expectationContext = null
    currentExpectationContext = null
  }

  private def verifyExpectations() {
    callLog foreach expectationContext.verify _

    if (!expectationContext.isSatisfied)
      reportUnsatisfiedExpectation(callLog, expectationContext)
  }

  private def reportUnexpectedCalls(): Unit = {
    var calls: List[String] = Nil
    unexpectedCalls.foreach {
      call => calls = s"Unexpected call: $call" :: calls
    }
    if (calls.nonEmpty) throw newExpectationException(calls.reverse.mkString("", "\n", s"\n\n${errorContext(callLog, expectationContext)}"))
  }
}

This workaround is pretty ugly but most of the code already exists in the MockFactoryBase and it works like a charm (so far). I only mix it into my test suite. This workaround both reports the unexpected calls directly as well as records them and at the end of a test, it first reports unexpected calls (if any) as they may influence the flow of a test and then verify the expectations. This works with a premise, that synchronous tests with an unexpected call are already failed with the original expection, so this workaround applies only to async tests with unexpected calls, otherwise it silently blends in.

[barkhorn]

If it's useful to people then we could add the MockFactoryRecordingUnexpectedCalls implementation as an alternative. If you can create a pull request then that will speed things up, otherwise i'll take a look as time permits.
The reason for not changing what you call the inconsistency is that the library has been around for some time now and people will have built their tests with that API behaviour in mind.

Don't get too hung up on mock vs stub terminology please. In Scalamock, the difference is that one is fail early while the other is fail late (or not fail at all, i you don't verify calls).