mockito "there was" does not fail test when used in a scope

[nadavwe]

Hi @etorreborre,
this is probably an old issue but I couldn't find the reference I was looking for when dealing with it.
It's actually a known myth in wix - sometimes mockito does not fail tests, beware! ^_^

so here is the code:

import org.specs2.mock.Mockito._
import org.specs2.mutable.SpecWithJUnit
import org.specs2.specification.Scope

trait Q {
  def call(i: Int): Int
}

class BlaTest extends SpecWithJUnit {
  "a" should {
    "b" in new Scope {
      val q = mock[Q]
      q.call(1) returns 1

      q.call(1) must be_==(1)
      q.call(1) must be_==(1)
      there was one(q).call(1)
    }
  }
}

you'd expect it to fail, but it passes just fine.
the problem is the Scope. once it's removed everything works as expected.
A workaround is adding .orThrow to the result of there was one(...), and that actually makes the test fail.

I guess that for some reason the match result from "there was" is not properly converted into an exception...? or something like that...?

we're using Specs2 4.19.1 with scala 2.12.

Thanks anyway!

[etorreborre]

Hi @nadavwe, this is all a bit tricky (thanks to exceptions :-)) but to make things work you need to have the whole specification inherit from Mockito

class BlaSpec extends org.specs2.mutable.SpecWithJUnit with Mockito {
...
}

then you get

[info] BlaSpec
[info] a should
[error]   x b
[error]    The mock was not called as expected:
[error]    q.call(1);

We want to make sure that was, which is a Mockito definition, is wired to throw exceptions. To do that it needs to come from the Mockito trait (and not the Mockito object) which takes its behaviour for throwing exceptions from additional traits mixed in the Specification. In this case, a SpecWithJUnit is always throwing exceptions for failures.

Another alternative is to define another Mockito object like that:

object MockitoThrownExpectations extends org.specs2.mockito.Mockito with org.specs2.matcher.ThrownExpectations
import MockitoThrownExpectations._

[nadavwe]

hey @etorreborre, thanks for the explanation, it clears things up! :)
I generally prefer to always use the objects rather than the mixins. this is because in mixins if the code changes and you don't use some methods anymore, the IDE does not know that you're bringing unused functionality...

regarding the alternative you offer: why isn't it the definition used in specs2 Mockito object? i.e. why doesn't org.specs2.mockito.Mockito use ThrownExpectations? (I guess it's because other code does not work properly with that...)

and a last question: basically you're closing this issue as won't fix? :)
is there anyway this can be fixed?
from a quick look at the code I would guess the reason for the different behavior is the with Expectations clause in the Mockito trait... Am I close, or not at all? ^_^

[etorreborre]

regarding the alternative you offer: why isn't it the definition used in specs2 Mockito object? i.e. why doesn't org.specs2.mockito.Mockito use ThrownExpectations? (I guess it's because other code does not work properly with that...)

That's because the default stance in specs2 (for better or worse) is to not throw exceptions.

from a quick look at the code I would guess the reason for the different behavior is the with Expectations clause in the Mockito trait... Am I close, or not at all? ^_^

You're totally right. Then it all depends on the concrete Expectations trait that is also mixed-in. If this is a ThrownExpectations trait then exceptions will be thrown for failures.

basically you're closing this issue as won't fix? :)
is there anyway this can be fixed?

The fix I can propose is to add the MockitoThrownExpectations object alongside the Mockito object. There are other examples of this with MustThrownMatchers for example, alongside MustMatchers.

[nadavwe]

That's because the default stance in specs2 (for better or worse) is to not throw exceptions.

for better probably ^_^

The fix I can propose is to add the MockitoThrownExpectations object alongside the Mockito object. There are other
examples of this with MustThrownMatchers for example, alongside MustMatchers.

that would be great, thanks!
the only bad thing is that this behavior stays and there is no warning... :(
but it sounds like it can happen in other places... people should always make sure their tests fail!

[etorreborre]

that would be great, thanks!

I'll try to do that later that week and maybe into the week-end.

the only bad thing is that this behavior stays and there is no warning... :(

Yes there are some cases where it is possible to raise such issues. For example there is a runtime check telling you that you tried to use a Scope trait with a specification which cannot throw exceptions. But in that case I don't see what can be done.

[nadavwe]

Cool, thanks!

another question about that...
does it mean that the object versions of, e.g, FutureMatchers or JsonMatchers also do not fail a test with a scope? 😱

[etorreborre]

Excellent question! In principle, any Matcher is used via must (or should) to create an Expectation.
The behaviour of throwing an exception is wired into must (via an inherited trait) and not into the matcher.

The problem we had with there was is that was was not being brought into scope via the Specification and its specific Expectations trait.

[nadavwe]

ohhh, right, I understand the difference now.
basically the entire thing is not even a matcher.

so essentially if we had some code like

mock must haveBeenCalledTwice

it would have worked....
that is super ugly (and not really working or even remotely doing what it should), but just making a point here.

[etorreborre]

I have now published it as version 4.20.3