mockito icon indicating copy to clipboard operation
mockito copied to clipboard

Mockito confuses varargs and single-param methods (regression)

Open lewisd32 opened this issue 6 years ago • 6 comments

In Mockito 2.2.5 and later, when setting expectations for mocking a varargs method, using an array doesn't work:

Mockito.when(good.call(Mockito.any(String[].class))).thenReturn(1L);

And instead the contained type must be used:

Mockito.when(good.call(Mockito.any(String.class))).thenReturn(1L);

However in the face of an overload of the varargs method that takes a single argument (of the same type):

    Long call(String value);
    Long call(String... values);

This results in being unable to set an expectation for the varargs call. This worked previously in Mockito 2.2.4 and earlier.

Reproducer: mockito-bug.tar.gz

lewisd32 avatar Jan 17 '19 16:01 lewisd32

I can fix it.

jjDevPL avatar Mar 13 '19 21:03 jjDevPL

try this as workaround: Mockito.when(good.call((String[])Mockito.any())).thenReturn(1L);

jjDevPL avatar Apr 29 '19 20:04 jjDevPL

the problem is matches method from internal class VarArgAware in InstanceOf. It need to be override.

jjDevPL avatar Apr 29 '19 20:04 jjDevPL

Hi @jjDevPL

Thanks for tracking the version that changed the behavior ! This was done as part of #635.

For other here's the code in the archive :

interface Good {
    Long call(String... values);
}

interface Bad {
    Long call(String value);
    Long call(String... values);
}

private Good good = Mockito.mock(Good.class);
private Bad bad = Mockito.mock(Bad.class);

private String value = "abc";

// Expectation with `String[]` and call vararg method with a single-element outer-array.
// This passes in Mockito 2.2.4 and earlier
// This fails in Mockito 2.2.5 and later
@Test
public void testMockitoVarArgs1() {
    Mockito.when(good.call(Mockito.any(String[].class))).thenReturn(1L);

    Long count = good.call(new String[]{value});
    assertEquals(1, (long) count);
}

// The failure above can be fixed by changing the expectation to `String` (with the exception of the case below)
// This passes all Mockito versions
@Test
public void testMockitoVarArgs2() {
    Mockito.when(good.call(Mockito.any(String.class))).thenReturn(1L);

    Long count = good.call(new String[]{value});
    assertEquals(1, (long) count);
}

// However, with `bad` having the single-arg method, the fix doesn't work, and there is no way I can find to make
// this pass.
// This fails in all Mockito versions
@Test
public void testMockitoVarArgs3() {
    Mockito.when(bad.call(Mockito.any(String.class))).thenReturn(1L);

    Long count = bad.call(new String[]{value});
    assertEquals(1, (long) count);
}

Both testMockitoVarArgs1 and testMockitoVarArgs3 are failing.

  1. Reagrding testMockitoVarArgs1, this is somehow expected, as it is a part of the enhancement of #635, however we probably missed to document this case. The stub should now be configured as :

    Mockito.when(good.call(Mockito.any(String.class))).thenReturn(1L);
    

    Note the any(String.class) not any(String[].class).

  2. The issue with testMockitoVarArgs3 is that the stub is resolved to Long call(String value) because any(String.class) returns a single argument. But the production code is compiled with the vararg signature Long call(String... values). Forcing the cast to String[] don't work as well.

    The only option is to use

    Mockito.when(bad.call((String[]) Mockito.anyVararg())).thenReturn(1L);
    

    Unfortunately this API is deprecated since 2.1.0, also the current return type do not indicate an array and thus forces the cast. Tweaking the vararg matcher signature can solve the issue

    <T> T[] anyVararg() // enough for Java 8 as compiler infers the type
    <T> T[] vararg(Class<T> type) // for earlier javac version
    

    In this case deprecation of this matcher should be lifted, as it this is the only solution that allows to work with ambiguous method declarations.


@jjDevPL This doesn't happen if the API of the type to mock is designed this way (which is better) :

Long call(String value, String..., moreValues)

I don't know if you own this API, but this could solves you issue faster than us.

bric3 avatar May 07 '19 10:05 bric3

well, the issue got bug label. There are work around but nevertheless it should be fixed. My changes are located in InstanceOf.java

protected Class getClazz() {
        return clazz;
    }

and under VarArgAware

  public boolean matches(Object actual) {
            return (actual != null) &&
                    (Primitives.isAssignableFromWrapper(actual.getClass(), getClazz())
                            || (getClazz().getComponentType() != null && getClazz().getComponentType().isAssignableFrom(actual.getClass()))
                            || getClazz().isAssignableFrom(actual.getClass()));
        }

small changes but it fixes Bad example. #635 was meged long time ago and I think Bad example is still bad.

jjDevPL avatar May 19 '19 20:05 jjDevPL

I am trying to mock querydsl orderBy method: public api methods on com.querydsl.core.support.QueryBase

public Q orderBy(OrderSpecifier<?>... o) 
public Q orderBy(OrderSpecifier<?> o)

the project is on java 1.8 yet, and the workaround using deprecated Mockito api worked, but java version change will break it.

padisah avatar Apr 25 '22 14:04 padisah

With #2835:

when(good.call(any(String[].class))).thenReturn(1L);

...now works, matching any number of values passed to the varargs parameter.

Where as:

when(good.call(any(String.class))).thenReturn(1L);

... will match exactly one value passed to the varargs parameter.

big-andy-coates avatar Dec 29 '22 05:12 big-andy-coates