paulbutcher / ScalaMock

Native Scala mocking framework
http://scalamock.org/
MIT License
502 stars 99 forks source link

Verifying the number of invocations is unreliable #444

Closed CalumMcCall closed 2 years ago

CalumMcCall commented 2 years ago

ScalaMock Version (e.g. 3.5.0)

5.2.0

Scala Version (e.g. 2.12)

2.13.8

Runtime (JVM or JS)

JVM v11

Please describe the expected behavior of the issue

I expect verify used with times to produce an error when the number of invocations is greater than, or less than the number passed to times.

Please provide a description of what actually happens

The test always passes, or invocations which are greater than the number passed to times passes. I've added a manual version of the invocation check to my test case using AtomicInteger, this was to test that my code really does execute the expected number of times.

Reproducible Test Case

    "retry when sendRequest returns a failed future" in {
      val uriPath = "/zero/retries"
      val req = mkHttpRequest(uriPath)
      val config = configNoRetries.copy(retries = 3)
      val mockHttpClient = mock[HttpClient]
      // use AtomicInteger to count invocations, because scalatest's method of doing this using `times()` is broken
      val invocationCount = new AtomicInteger(0)
      (mockHttpClient
        .sendRequest(_: HttpRequest)(_: ActorSystem, _: ExecutionContext))
        .expects(req, *, *)
        .onCall { _ =>
          invocationCount.incrementAndGet()
          Future.failed(new Throwable("client timed out"))
        }
        .anyNumberOfTimes()

      val client = new RetryableFranzClient(config, mockHttpClient)

      val response = client.sendRequest(req)

      whenReady(response.failed) { _ =>
        assert(invocationCount.get == 4, "http call must be invoked 4 times")
        verify(mockHttpClient, times(2)).sendRequest(_: HttpRequest)(_: ActorSystem, _: ExecutionContext)
      }

My test class is inheriting from MockFactory in case that's important.

Of course, I seriously doubt the methods don't work as intended, I assume the issue is with my setup somehow. I am using Mockito as well as scalacheck too in other tests, maybe there is some kind of inteference?

barkhorn commented 2 years ago

I see some references to ActorSystem in there and Futures. For async tests, you need to use a different MockFactory. Check out this example here: https://github.com/paulbutcher/ScalaMock/blob/master/shared/src/test/scala/org/scalamock/test/scalatest/BasicAsyncTest.scala