withObjectMocked is not threadsafe

[oleksii-suprun]

Hi! It seems withObjectMocked may cause unexpected failures when tests are executing in parallel. According to implementation, it uses reflection to set an internal state of a singleton which may lead to unexpected behavior when another test runs in a different thread and tries to interact with that scala object.

This can lead to the following exceptions:

org.mockito.exceptions.verification.NoInteractionsWanted: No interactions wanted here

or

org.mockito.exceptions.misusing.UnexpectedInvocationException: Unexpected invocations found

or

org.mockito.exceptions.verification.SmartNullPointerException: You have a NullPointerException here

Any help or workaround will be appreciated! Thanks in advance.

[ultrasecreth]

Interesting, according to the java docs of the feature it relies on it should be thread safe if used properly

 * <h3 id="48">48. <a class="meaningful_link" href="#static_mocks" name="static_mocks">Mocking static methods</a> (since 3.4.0)</h3>
 *
 * When using the <a href="#0.2">inline mock maker</a>, it is possible to mock static method invocations within the current
 * thread and a user-defined scope. This way, Mockito assures that concurrently and sequentially running tests do not interfere.
 *
 * To make sure a static mock remains temporary, it is recommended to define the scope within a try-with-resources construct.
 * In the following example, the <code>Foo</code> type's static method would return <code>foo</code> unless mocked:
 *
 * <pre class="code"><code class="java">
 * assertEquals("foo", Foo.method());
 * try (MockedStatic<Foo> mocked = mockStatic(Foo.class)) {
 * mocked.when(Foo::method).thenReturn("bar");
 * assertEquals("bar", Foo.method());
 * mocked.verify(Foo::method);
 * }
 * assertEquals("foo", Foo.method());
 * </code></pre>
 *
 * Due to the defined scope of the static mock, it returns to its original behavior once the scope is released. To define mock
 * behavior and to verify static method invocations, use the <code>MockedStatic</code> that is returned.
 * <p>

I wonder if there is anything wrong in my impl...

[oleksii-suprun]

@bbonanno thank you for your response.

I think there is an issue with the following piece of code in the current implementation:

      ReflectionUtils.setFinalStatic(moduleField, mock[O])

The piece of doc you referenced describes a usage of a special case of mocking org.mockito.Mockito#mockStatic(java.lang.Class<T>). But in the current mockito-scala uses the following method org.mockito.MockitoEnhancer#mock(scala.reflect.ClassTag<T>, scala.reflect.api.TypeTags.WeakTypeTag<T>, org.mockito.stubbing.DefaultAnswer, org.scalactic.Prettifier), which is, according to the doc, just delegates call to org.mockito.Mockito#mock(java.lang.Class<T>) (for non-static cases):

   * Delegates to <code>Mockito.mock(type: Class[T])</code>
   * It provides a nicer API as you can, for instance, do <code>mock[MyClass]</code>
   * instead of <code>mock(classOf[MyClass])</code>

[ultrasecreth]

Ohh yes, you're right.
I actually got the inspiration from the fact the core guys released that method, but is of no use for Scala, you see, an object is not a collection of static methods, but just simply a singleton pattern, so you have a static final MODULE$ variable that holds a single instance of the object class.
So what I had to do wasn't mocking a static method, but basically replacing that instance with a mock, and when the code block finishes swap it back by the original one.

So yes, is non thread-safe as it stands now, I'll try to see if I can make it so, but will probably have to involve a lock on that class or something (which will prevent tests that use that object to run in parallel, but unless your whole system is calling the same object it should be fine I think).

Gonna sketch something out and see if I can push it today

[ultrasecreth]

@aleksey-suprun please check the PR associated to this issue, the solution is a bit nuclear, but I don't think there's a way around it as we can't control how the compiler links the access to the singleton instance

[oleksii-suprun]

Hi @bbonanno. I checked your solution. Actually, my first attempt to work the problem around was exactly the same. But it does not solve the main issue. The problem is not connected with a parallel mocking. The problem pops up when a number of tests interact with the same scala object (singleton) in parallel. Due to the fact that scala object is a singleton, we are getting mocked instance even in tests which are not going to mock the object but run in parallel in a separate thread. That causes some unverified calls in mocked tests and smart nulls exceptions in non-mocked tests during the execution. For example, we have a couple of test classes:

class FooSpec extends AnyFunSpec with MockitoSugar {

  it ("should use mock") {
    withObjectMocked[FooObject.type] {
      when(FooObject.simpleMethod).thenReturn(1)
      FooObject.simpleMethod
      verify(FooObject).simpleMethod
    }
  }
}

class BarSpec extends AnyFunSpec with MockitoSugar {

  it ("should not use mock") {
    FooObject.simpleMethod should not be (null)
  }
}

There is a high probability to get both of them failed when they run simultaneously in separate threads.

[ultrasecreth]

ohh I see, mixed concurrency would definitively be a problem...
Let me put a test together that checks that and see if there is a way around it

[oleksii-suprun]

Thank you. Today I tried to find something which can help but still don't have any solution.

[ultrasecreth]

@aleksey-suprun please have a look again, I think I got it right this time :)

[ultrasecreth]

booo, it worked in my machine 😅

[ultrasecreth]

OK, this time I got a green build :P

Please let me know your thoughts @aleksey-suprun

[oleksii-suprun]

@bbonanno that looks great! 👍 Looking forward to the release 😉