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

Use generic type information in TypeBasedCandidateFilter to circumvent type erasure #2921

Closed
jfrantzius opened this issue Feb 21, 2023 · 10 comments · Fixed by #2923
Closed

Comments

@jfrantzius
Copy link
Contributor

Hi,
when using @InjectMocks in combination with multiple @Mock fields that have the same generic type (after type erasure), this fails unless the field names in injectee and test class are equal.

I stumbled over this when I added a second Provider<T> field to both test and injectee class, and I guess most other people doing the same will also fail here as miserably as I did (also because googling for @InjectMock doesn't show the relevant InjectMocks documentation in the first result page ...)

E.g. this test will fail because I named the test class @Mock fields with the suffix mock. When deleting that name suffix, the test succeeds.

import static org.junit.jupiter.api.Assertions.assertNotNull;

import javax.inject.Provider;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
public class InjectMockTest {
    
    public static class UnderTest {
        Provider<String> stringProvider;
        Provider<Integer> intProvider;
    }

    @Mock
    private Provider<String> stringProviderMock;

    @Mock
    private Provider<Integer> intProviderMock;

    @InjectMocks
    private UnderTest underTest;

    @Test
    public void testInjectMock() {
        assertNotNull(underTest.stringProvider);
        assertNotNull(underTest.intProvider);
    }

}

I saw in the debugger that in TypeBasedCandidateFilter.filterCandidate() , candidateFieldToBeInjected.getGenericType().getTypeName() yields e.g. "javax.inject.Provider<java.lang.Integer>". So if Mockito would maintain the @Mock field's java.lang.reflect.Field along with the mock object, the generic type names could be compared.

This would effectively solve the problem of type erasure for this usecase, which I'd guess is quite common ...

@TimvdLippe
Copy link
Contributor

Duplicate of #1066

@TimvdLippe TimvdLippe marked this as a duplicate of #1066 Feb 21, 2023
@jfrantzius
Copy link
Contributor Author

Hi @TimvdLippe , my suggestion is not about using the name parameter of the @Mock annotation, but using Field type information to perform the match.

So with my suggestion, there would be no need to use the above at all or having to align field names in the first place.

jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 22, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 22, 2023
@jfrantzius
Copy link
Contributor Author

jfrantzius commented Feb 22, 2023

@TimvdLippe please find PR #2923 to prove the point

@jfrantzius
Copy link
Contributor Author

jfrantzius commented Feb 25, 2023

Hi @TimvdLippe , I saw this in https://github.com/mockito/mockito/actions/runs/4244476122/jobs/7425248431 :

[Undefined reference | android-api-level-26-8.0.0_r2] org.mockito.internal.configuration.injection.filter.(TypeBasedCandidateFilter.java:36)
  >> String java.lang.reflect.Type.getTypeName()

Does Mockito want to maintain support down to Android API level 26?

The method in question was added in Android API level 28: https://developer.android.com/reference/java/lang/reflect/Type#getTypeName()

@TimvdLippe
Copy link
Contributor

Yes we need to maintain support for level 26 for our Android userbase.

jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 26, 2023
@jfrantzius
Copy link
Contributor Author

Please have a look @TimvdLippe

jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 26, 2023
@jfrantzius
Copy link
Contributor Author

I fixed formatting, could you please approve workflow @TimvdLippe ?

jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 27, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 27, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 27, 2023
@jfrantzius
Copy link
Contributor Author

@TimvdLippe it now also works for Spys and supports matching things like TreeSet<String> to Set<?>

jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 27, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 27, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 27, 2023
@jfrantzius
Copy link
Contributor Author

@TimvdLippe spotless check shouldn't fail anymore now, could you please start the workflow again?

@jfrantzius jfrantzius changed the title TypeBasedCandidateFilter could match field type signature to circumvent type erasure User generic type information in TypeBasedCandidateFilter to circumvent type erasure Feb 27, 2023
@jfrantzius jfrantzius changed the title User generic type information in TypeBasedCandidateFilter to circumvent type erasure Use generic type information in TypeBasedCandidateFilter to circumvent type erasure Feb 27, 2023
@jfrantzius
Copy link
Contributor Author

@TimvdLippe PR build #2923 was successful, is there anything that you'd miss in the PR?

jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 28, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 28, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 28, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Feb 28, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 6, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 6, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 6, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 7, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 7, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 7, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 8, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 8, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 8, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 8, 2023
jfrantzius pushed a commit to jfrantzius/mockito that referenced this issue Mar 9, 2023
TimvdLippe pushed a commit that referenced this issue Mar 9, 2023
Fixes #2921

Co-authored-by: Jörg von Frantzius <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants