Default mock of Optional is not empty when using RETURN_DEEP_STUBS

[big-andy-coates]

Version affected: Mockito: 4.x & 5.x

If you create a mock with RETURN_DEEP_STUBS, then any method that returns a collection will, by default, return an empty collection, i.e. isEmpty returns true, etc. This is great, as it means no additional configuration of the mock is required before passing to logic that would attempt to access elements of the collection.

However, the same is not true for Optional, (and I'm assuming OptionalLong etc): if RETURN_DEEP_STUBS is used, methods that return Optional aren't treated in any special way and just return a default mock with not stubbing configured. This means isEmpty returns false, but get() returns null. This breaks the contract of Optional, causing test failures that aren't actually valid, or requiring manual configuration of the Optional mock to do-the-right-thing, cluttering test code.

Here's an example test that demonstrates this:

interface Address {
   Optional<String> getState();
}

@Test
public void shouldDefaultDeepMockOptionalsToEmpty() {
    final Address address = mock(Address.class, RETURNS_DEEP_STUBS);

    assertThat(address.getState()).isEqualTo(Optional.empty());  // <--- fails
}

@Test
public void shouldDefaultMockOptionalsToEmpty() {
    final Address address = mock(Address.class); // <-- no RETURNS_DEEP_STUBS

    assertThat(address.getState()).isEqualTo(Optional.empty());  // <--- passes
}

Note the second test, which doesn't use deep mocking, passes.

• The mockito message in the stacktrace have useful information, but it didn't help
• The problematic code (if that's possible) is copied here;
  Note that some configuration are impossible to mock via Mockito
• Provide versions (mockito / jdk / os / any other relevant information)
• Provide a Short, Self Contained, Correct (Compilable), Example of the issue
  (same as any question on stackoverflow.com)
• Read the contributing guide

[TimvdLippe]

This should have been covered in

mockito/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsEmptyValues.java

Line 132 in 0aba0d5

} else if ("java.util.Optional".equals(type.getName())) {

but I am not sure why that is breaking. #2731 is supposed to tackle that, but it needs to be rebased. Feel free to submit a PR that fixes this issue!

[schlosna]

I'm seeing a similar test failures with deep stubs and methods returning Optional<Long> and OptionalLong after the upgrade to Mockito 5.x in palantir/tritium#1635 . The exception we see is below:

java.lang.ClassCastException: class com.github.benmanes.caffeine.cache.Policy$Eviction$MockitoMock$nsH7a8Ju cannot be cast to class java.lang.Long (com.github.benmanes.caffeine.cache.Policy$Eviction$MockitoMock$nsH7a8Ju is in unnamed module of loader 'app'; java.lang.Long is in module java.base of loader 'bootstrap')
	at com.palantir.tritium.metrics.caffeine.CaffeineStats.lambda$weightedSize$1(CaffeineStats.java:60)
	at com.palantir.tritium.metrics.caffeine.CaffeineCacheMetricsTest.lambda$weightedSize$2(CaffeineCacheMetricsTest.java:87)
	at org.assertj.core.api.AbstractObjectAssert.returns(AbstractObjectAssert.java:1106)
	at com.palantir.tritium.metrics.caffeine.CaffeineCacheMetricsTest.weightedSize(CaffeineCacheMetricsTest.java:87)

[big-andy-coates]

@schlosna, as far was I'm aware, this was not a regression, just an existing issue, effecting at least the latest 4.x and 5.x versions.

What version were you updating from?

[schlosna]

@schlosna, as far was I'm aware, this was not a regression, just an existing issue, effecting at least the latest 4.x and 5.x versions.

What version were you updating from?

I saw this in palantir/tritium#1635 when upgrading from Mockito 4.11.0 to 5.0.0 and running on JDK 17.

I tried dropping the tests above into a mockito clone locally, but they did not reproduce. I can try a few more things to try to reproduce.

[krzyk]

@big-andy-coates I tried to fix this but eventually I came across an issue that if you have RETURNS_DEEP_STUBS, why would you expect an Optional to be not a mock? If it wouldn't be you would make some users quite surprised.

Having List.isEmpty = false is a happy coincidence, because it is a mock and not a Collection.emptyList nor List.of().

In case of Optional, you get one that is not Optional.empty(), but gives you false when you ask for Optional.isPresent, the only issue is that it also returns false when you ask Optional.isEmpty - but this is normal mock behavior, defaults are set to false for booleans.

[big-andy-coates]

Should we consider leaving it as a mock, but stubbing the isPresent method to return false? This would require the mock to be lenient, but I think that's preferable to the current non-sensical defaults.

[AndreasTu]

The PR #3097 would now return true for all Optional.isEmpty() classes when using RETURNS_DEEP_STUBS and will return a real Optional also in the RETURNS_DEEP_STUBS, as discussen with @TimvdLippe in the PR.

The behavior of Collection.isEmpty() remains the same.