Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support For Generics Inference In Mockito mock() and spy() method #1531

Open
ankurpathak opened this issue Nov 14, 2018 · 12 comments
Open

Comments

@ankurpathak
Copy link

EasyMock with its version 4 added support for Generics to get rid of Annoying Unchecked Warning while
mocking types with Generics Involved. So we expect the same with Mockito, in its mock() and spy() to add
support for generics type inference, so that the developers have not to deal with annoying Uncheked
warning in Test and annotate tests with SuppressWarnings({"unchecked"}). For a motivation for same we
can have a look at emptyList(), empthMap() and emptySet() method of Collections class in JDK.
And also have a look at the way it is done in EasyMock 4.0 in this class:
https://github.com/easymock/easymock/blob/master/core/src/main/java/org/easymock/EasyMock.java

@brachi-wernick
Copy link

brachi-wernick commented Dec 11, 2018

@ankurpathak if you will check what EasyMock did, they make the createMock function not strongly typed.
means, you really can do

  BaseGenericClass<Integer> mock = EasyMock.createMock(BaseGenericClass.class);

but you can also do

    BaseGenericClass<Integer> mock = EasyMock.createMock(Set.class);

very strange, ha?!

I'll try to give a better fix...

@SerialVelocity
Copy link

@brachi-wernick This is already done for ArgumentCaptor:

public static <U,S extends U> ArgumentCaptor<U> forClass(Class<S> clazz) {
return new ArgumentCaptor<U>(clazz);
}

It doesn't make it completely typed as you can accidentally give it Object or some inherited class but it is a nice compromise :)

@bric3
Copy link
Contributor

bric3 commented Feb 26, 2019

Hi,

While it is not exactly what you are looking for, please note that at this time, if the types to mock are declared as fields in a test, then the compiler will propagate types.

import org.junit.Rule;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;

import java.util.Map;

import static java.lang.Long.MAX_VALUE;
import static java.util.Collections.singleton;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.BDDMockito.given;

public class GenericFieldTest {

    @Rule
    public MockitoRule rule = MockitoJUnit.rule();

    @Mock
    private Map<Number, Iterable<String>> typeToMock;

    @Test
    public void should_propagate_generics() {
        given(typeToMock.get(MAX_VALUE)).willReturn(singleton("max"));
        given(typeToMock.put(MAX_VALUE, singleton("max"))).willReturn(singleton("maximum"));

        assertThat(typeToMock.put(MAX_VALUE, singleton("max"))).isEqualTo(singleton("maximum"));
        assertThat(typeToMock.get(MAX_VALUE)).isEqualTo(singleton("max"));
    }
}

Also we discussed about the trick that easy mock used, and we're not really sold to it because of the drawbacks mentioned above. Some other libraries employs other approaches that are safer but require a stronger commitment to the codebase.

@thandy1212
Copy link

thandy1212 commented May 29, 2020

@SerialVelocity why does ArgumentCaptor have two type parameters? That allows something like the following doesn't it:

ArgumentCaptor<Object> ac = ArgumentCaptor.forClass(Integer.class);

which isn't semantically accurate. That's saying you're trying to declare an ArgumentCaptor that can take anything from an ArgumentCaptor that can only take Integers

If you try something similar with Lists it wouldn't compile:

// Won't compile
List<Object> ol = new ArrayList<Integer>();

@SerialVelocity
Copy link

@thandy1212 I'm just stating what already exists in Mockito, not something new.

I assume it was done because you don't really care about what subclass it is implementing. If you pass ac.capture() into something that accepts an object, so be it. All you've done is limited the type you get out when checking what value was passed in. e.g. In the current world, you can do:

void bob(Object o) {}

ArgumentCaptor<Integer> ac = ArgumentCaptor.forClass(Integer.class);
bob(ac.capture());

It's the same as if you had:

void bob(Object o) {}

ArgumentCaptor<Object> ac = ArgumentCaptor.forClass(Integer.class);
bob(ac.capture());

The only thing you have restricted is you cannot call void bob(Integer i) {} anymore.

@mjustin
Copy link

mjustin commented Sep 22, 2020

As of this writing, there are 292 upvotes for the related Stack Overflow question around mocking with generic parameters. This indicates to me that there is definitely an interest in having a clean solution to this problem.

@TimvdLippe
Copy link
Contributor

Given that .class does not play well with generics, I don't think there is any solution we can do in Mockito. You can already do @Mock (the JUnit5 extension also allows method parameter @Mocks) and that should be a suitable alternative. Therefore, we can keep this issue open, but it is unlikely ever to be fixed, given that @Mock is a better API.

@mjustin
Copy link

mjustin commented Sep 22, 2020

A problem I've seen with @Mock is when there's code which dynamically creates a mock, or wants it to be method scoped. In this case, a field can be problematic.

@TimvdLippe
Copy link
Contributor

Since you are dynamically creating the mock, it means you are creating it on runtime. Because of type erasure, at runtime these generics types are not checked. Therefore, I don't think it is possible to create a type-safe API with .class.

@mjustin
Copy link

mjustin commented Sep 22, 2020

@TimvdLippe Agreed, regarding using Class. What I've seen other libraries do is use a parameterized type reference class for this purpose (based on an approach by Neal Gafter).

Examples:

The code could then look something like:

List<String> mock = mock(new TypeReference<List<String>>(){});

@JoshMcCullough
Copy link

JoshMcCullough commented Oct 18, 2023

I don't like ignoring warnings and I hate adding warning-ignore comments in code (they don't belong there). Please help!

matthieu-vergne added a commit to matthieu-vergne/javaparser that referenced this issue May 5, 2024
This is an issue well known by the Mockito community, but prone to not
be fixed:
mockito/mockito#1531

Generics allow to ensure type-safety at compile time instead of runtime.
However, here it is used mainly to auto-cast the created object, thus
avoiding this burden on the developer. Indeed, it is in a context where
type safety is usually not a requirement, since the use of this method
is often the most trivial we can have: provide a class and expect a very
instance of this class in return.

This commit thus creates a less constrained mock method which builds on
Mockito's one, but without the constraint inducing this warning.
matthieu-vergne added a commit to matthieu-vergne/javaparser that referenced this issue May 6, 2024
This is an issue well known by the Mockito community, but prone to not
be fixed:
mockito/mockito#1531

Generics allow to ensure type-safety at compile time instead of runtime.
However, here it is used mainly to auto-cast the created object, thus
avoiding this burden on the developer. Indeed, it is in a context where
type safety is usually not a requirement, since the use of this method
is often the most trivial we can have: provide a class and expect a very
instance of this class in return.

This commit thus creates a less constrained mock method which builds on
Mockito's one, but without the constraint inducing this warning.
@jlerbsc
Copy link

jlerbsc commented May 6, 2024

Hi, Have you planned to resolve this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants