I used doReturn, why would Mockito still call real implementation inside anonymous class?

Question

Class I want to test:

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;

public class Subject {

    private CacheLoader<String, String> cacheLoader = new CacheLoader<String, String>() {
        @Override
        public String load(String key)
                throws Exception {
            return retrieveValue(key);
        }
    };

    private LoadingCache<String, String> cache = CacheBuilder.newBuilder()
            .build(cacheLoader);

    public String getValue(String key) {
        return cache.getUnchecked(key);
    }

    String retrieveValue(String key) {
        System.out.println("I should not be called!");
        return "bad";
    }
}

Here's my test case

import static org.junit.Assert.assertEquals;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.doReturn;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Spy;
import org.mockito.runners.MockitoJUnitRunner;

@RunWith(MockitoJUnitRunner.class)
public class SubjectTest {

    String good = "good";

    @Spy
    @InjectMocks
    private Subject subject;

    @Test
    public void test() {
        doReturn(good).when(subject).retrieveValue(anyString());
        assertEquals(good, subject.getValue("a"));
    }
}

I got

org.junit.ComparisonFailure: 
Expected :good
Actual   :bad

Answer 1

This comes down to the implementation of the spy. According to the docs, the Spy is created as a copy of the real instance:

Mockito does not delegate calls to the passed real instance, instead it actually creates a copy of it. So if you keep the real instance and interact with it, don't expect the spied to be aware of those interaction and their effect on real instance state. The corollary is that when an unstubbed method is called on the spy but not on the real instance, you won't see any effects on the real instance.

It seems to be a shallow copy. As a result, as far as my debugging shows, the CacheLoader is shared between the copy and the original object, but its reference to its enclosing object is the original object, not the spy. Therefore the real retrieveValue is called instead of the mocked one.

I'm not sure offhand what the best way to resolve this would be. One way for this specific example would be to invert the CacheLoader dependency (i.e. pass it into Subject instead of Subject defining it internally), and mock that instead of Subject.

Answer 2

Mark Peters did a great job diagnosing and explaining the root cause. I can think of a couple workarounds:

• Move cache (re)initialization into a separate method.

  By calling new CacheLoader from within the spy, the anonymous inner class is created with a reference to the spy as the parent instance. Depending on your actual system under test, you may also benefit from getting the cache creation out of the constructor path, especially if there's any heavy initialization or loading involved.

  public class Subject {
  
    public Subject() {
      initializeCache();
    }
  
    private LoadingCache<String, String> cache;
  
    @VisibleForTesting
    void initializeCache() {
      cache = CacheBuilder.newBuilder().build(new CacheLoader<String, String>() {
        @Override
        public String load(String key) throws Exception {
          return retrieveValue(key);
        }
      });
    }
  
    /* ... */
  }
  
  @Test
  public void test() {
    subject.initializeCache();
    doReturn(good).when(subject).retrieveValue(anyString());
    assertEquals(good, subject.getValue("a"));
  }
  

• Make a manual override.

  The root cause of your trouble is that the spy instance is different from the original instance. By overriding a single instance in your test, you can change behavior without dealing with the mismatch.

  @Test
  public void test() {
    Subject subject = new Subject() {
      @Override public String getValue() { return "good"; }
    }
  }
  

• Refactor.

  Though you can go for full DI, you may be able to just add a testing seam to the value function:

  public class Subject {
  
    private CacheLoader<String, String> cacheLoader = new CacheLoader<String, String>() {
      @Override
      public String load(String key) throws Exception {
        return valueRetriever.apply(key);
      }
    };
  
    private LoadingCache<String, String> cache =
        CacheBuilder.newBuilder().build(cacheLoader);
  
    Function<String, String> valueRetriever = new Function<String, String>() {
      @Override
      public String apply(String t) {
        System.out.println("I should not be called!");
        return "bad";
      }
    };
  
    public String getValue(String key) {
      return cache.getUnchecked(key);
    }
  }
  
  @Test
  public void test() {
    subject = new Subject();
    subject.valueRetriever = (x -> good);
    assertEquals(good, subject.getValue("a"));
  }
  

  Naturally, depending on your needs, valueRetriever could be an entirely separate class, or you could accept an entire CacheLoader as a parameter.