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

MethodMatcher can fail for restatement of generic method #15

Open
dmcg opened this issue Dec 15, 2012 · 3 comments
Open

MethodMatcher can fail for restatement of generic method #15

dmcg opened this issue Dec 15, 2012 · 3 comments
Assignees

Comments

@dmcg
Copy link
Contributor

dmcg commented Dec 15, 2012

A bit subtle, but definitely confusing. If you reify a generic method in a sub-interface, then Java considers that a new declaredMethod. MethodMatcher only considers if the method called is .equals, rather than compatible...

... although I really don't know what compatible means. Anyhoo, this demonstrates the problem -

import org.jmock.Expectations;
import org.jmock.integration.junit4.JUnitRuleMockery;
import org.junit.Rule;
import org.junit.Test;
import static org.junit.Assert.assertEquals;

public class JMockMethodMatchingTest
{
    @Rule public JUnitRuleMockery context = new JUnitRuleMockery();

    private static interface Function<F, T> {
        T apply(F input);
    }

    private static interface StringFunction extends Function<String, String> {
        @Override public String apply(String input);
    }

    private Function<String, String> mockedFunction = context.mock(Function.class);
    private StringFunction mockedStringFunction = context.mock(StringFunction.class);

    @Test
    public void test() throws Exception
    {
        context.checking(new Expectations() {{
            allowing (mockedStringFunction).apply("alice");
                will (returnValue("bob"));
            allowing (mockedFunction).apply("alice");
                will (returnValue("bob"));
        }});

        assertEquals("bob", mockedFunction.apply("alice"));
        assertEquals("bob", mockedStringFunction.apply("alice"));

        assertEquals("bob", apply(mockedFunction, "alice"));
        assertEquals("bob", apply(mockedStringFunction, "alice"));
            // java.lang.AssertionError: unexpected invocation: stringFunctor.apply("alice")
    }

    private String apply(Function<String, String> f, String arg) {
        return f.apply(arg);
    }
}
@npryce
Copy link
Member

npryce commented Dec 16, 2012

Explanation:

The full type is used to make the invocation that captures the expected method, and therefore the expectation captures the override/instantiation of the generic method.

However, at runtime when called through the generic type, all generic parameters are actually treated as static type Object (thanks to type erasure). Concrete generic types have synthetic "bridge" methods that cast their Object-type parameters to the correct type and call the implementation. But bridge methods cannot be generated for interfaces.

So, jMock will have to implement bridge methods itself, somehow. I don't think this can be done in the chain of Invokable objects created by the Mockery to implement basic object semantics for mock objects. So maybe it needs to be done when the expectation is created -- if the captured method is an instantiation of a generic method, mock the generic method as well.

@dmcg
Copy link
Contributor Author

dmcg commented Dec 27, 2012

So I haven't had time to think through potential false positives, but MethodMatcher could be smarter to solve this problem? Something like

public class MethodMatcher extends TypeSafeMatcher<Method> {
    private Method expectedMethod;

    ...

    @Override
    public boolean matchesSafely(Method m) {
        if (expectedMethod.equals(m))
            return true;
        if (!Modifier.isAbstract(expectedMethod.getModifiers()) || !Modifier.isAbstract(m.getModifiers()))
            return false;
        if (!expectedMethod.getName().equals(m.getName()))
            return false;
        if (!m.getDeclaringClass().isAssignableFrom(expectedMethod.getDeclaringClass()))
            return false;
        if (!m.getReturnType().isAssignableFrom(expectedMethod.getReturnType()))
            return false;
        if (!parametersAssignable(expectedMethod, m))
            return false;
        return true;
    }

    private boolean parametersAssignable(Method m1, Method m2) {
        if (m1.getParameterTypes().length != m2.getParameterTypes().length)
            return false;
        for (int i = 0; i < m2.getParameterTypes().length; i++) {
            if (!m2.getParameterTypes()[i].isAssignableFrom(m2.getParameterTypes()[i]))
                return false;
        }
        return true;
    }
}

which passes all current tests and mine.

@olibye
Copy link
Member

olibye commented May 12, 2015

Check resolved by #38

@olibye olibye self-assigned this May 12, 2015
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

3 participants