From 8b03c0011b5fd9e10df55e0a3fcd2cd525346eeb Mon Sep 17 00:00:00 2001 From: Duncan McGregor Date: Mon, 31 Dec 2012 12:31:30 +0000 Subject: [PATCH 1/2] Putative fix for issues/15 MethodMatcher can fail for restatement of generic method Allow MethodMatcher to match some likely matching methods --- .../jmock/internal/matcher/MethodMatcher.java | 44 ++++++- .../GenericInvocationAcceptanceTests.java | 118 ++++++++++++++++++ 2 files changed, 159 insertions(+), 3 deletions(-) create mode 100644 test/org/jmock/test/acceptance/GenericInvocationAcceptanceTests.java diff --git a/src/org/jmock/internal/matcher/MethodMatcher.java b/src/org/jmock/internal/matcher/MethodMatcher.java index 5ec49d4de..bf8303e6e 100644 --- a/src/org/jmock/internal/matcher/MethodMatcher.java +++ b/src/org/jmock/internal/matcher/MethodMatcher.java @@ -1,6 +1,7 @@ package org.jmock.internal.matcher; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import org.hamcrest.Description; import org.hamcrest.TypeSafeMatcher; @@ -12,12 +13,49 @@ public MethodMatcher(Method expectedMethod) { super(Method.class); this.expectedMethod = expectedMethod; } - + +// @Override +// public boolean matchesSafely(Method m) { +// System.out.format("Expected %s, actual %s\n", expectedMethod, m); +// return expectedMethod.equals(m); +// } + @Override public boolean matchesSafely(Method m) { - return expectedMethod.equals(m); + System.out.format("Expected %s, actual %s\n", expectedMethod, m); + if (expectedMethod.equals(m)) + return true; + if (!expectedMethod.getName().equals(m.getName())) + return false; + if (!areImplementingInterfaces(expectedMethod.getDeclaringClass(), m.getDeclaringClass())) + return false; + if (!(Modifier.isAbstract(expectedMethod.getModifiers()) && Modifier.isAbstract(m.getModifiers()))) + return false; + if (!m.getReturnType().isAssignableFrom(expectedMethod.getReturnType())) + return false; + if (!parametersAssignable(expectedMethod, m)) + return false; + return true; } - + + private boolean areImplementingInterfaces(Class expectedClass, Class actualClass) { + if (!actualClass.isAssignableFrom(expectedClass)) + return false; + if (!(actualClass.isInterface() && expectedMethod.getDeclaringClass().isInterface())) + 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; + } + @Override protected void describeMismatchSafely(Method m, Description mismatchDescription) { mismatchDescription.appendText("was ").appendText(m.getName()); diff --git a/test/org/jmock/test/acceptance/GenericInvocationAcceptanceTests.java b/test/org/jmock/test/acceptance/GenericInvocationAcceptanceTests.java new file mode 100644 index 000000000..08046070d --- /dev/null +++ b/test/org/jmock/test/acceptance/GenericInvocationAcceptanceTests.java @@ -0,0 +1,118 @@ +package org.jmock.test.acceptance; + +import org.jmock.Expectations; +import org.jmock.integration.junit4.JUnitRuleMockery; +import org.jmock.lib.legacy.ClassImposteriser; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import static org.junit.Assert.assertEquals; + +public class GenericInvocationAcceptanceTests +{ + @Rule public JUnitRuleMockery context = new JUnitRuleMockery(); + + private static interface Function { + T apply(F input); + } + + private static interface StringFunction extends Function { + @Override public String apply(String input); + } + + private static abstract class AbstractStringFunction implements Function { + @Override public abstract String apply(String input); + } + + private static String apply(Function f, String arg) { + return f.apply(arg); + } + + @SuppressWarnings("unchecked") + private static Object objectApply(Function f, Object arg) { + return f.apply(arg); + } + + @Test + public void plain_old_invocation() + { + @SuppressWarnings("unchecked") + final Function f = context.mock(Function.class); + + // NB - don't try to remove this duplication - the resulting erasure invalidates the tests + context.checking(new Expectations() {{ + allowing(f).apply("alice"); + will (returnValue("bob")); + }}); + assertEquals("bob", f.apply("alice")); + assertEquals("bob", apply(f, "alice")); + assertEquals("bob", objectApply(f, "alice")); + } + + @Test + public void invocation_of_overriden_class_method() + { + context.setImposteriser(ClassImposteriser.INSTANCE); + final AbstractStringFunction f = context.mock(AbstractStringFunction.class); + + // NB - don't try to remove this duplication - the resulting erasure invalidates the tests + context.checking(new Expectations() {{ + allowing(f).apply("alice"); + will (returnValue("bob")); + }}); + assertEquals("bob", f.apply("alice")); + assertEquals("bob", apply(f, "alice")); + assertEquals("bob", objectApply(f, "alice")); + } + + @Test + public void invocation_of_overriden_interface_method_with_capture_of_generic_interface() + { + final Function f = context.mock(StringFunction.class); + + // NB - don't try to remove this duplication - the resulting erasure invalidates the tests + context.checking(new Expectations() {{ + allowing(f).apply("alice"); + will (returnValue("bob")); + }}); + assertEquals("bob", f.apply("alice")); + assertEquals("bob", apply(f, "alice")); + assertEquals("bob", objectApply(f, "alice")); + } + + @Test + public void invocation_of_overriden_interface_method_with_capture_of_concrete_interface() + { + final StringFunction f = context.mock(StringFunction.class); + + // NB - don't try to remove this duplication - the resulting erasure invalidates the tests + context.checking(new Expectations() {{ + allowing(f).apply("alice"); + will (returnValue("bob")); + }}); + assertEquals("bob", f.apply("alice")); + assertEquals("bob", apply(f, "alice")); + assertEquals("bob", objectApply(f, "alice")); + } + + @Test + @Ignore("Fails, and cure might be worse than the disease?") + public void invocation_of_overriden_interface_method_with_capture_via_function() + { + final StringFunction f = context.mock(StringFunction.class); + + expectApply(f, "alice", "bob"); + + assertEquals("bob", f.apply("alice")); + assertEquals("bob", apply(f, "alice")); + assertEquals("bob", objectApply(f, "alice")); + } + + private void expectApply(final Function f, final String arg, final String result) { + context.checking(new Expectations() {{ + allowing(f).apply(arg); + will (returnValue(result)); + }}); + } + +} \ No newline at end of file From 6a0c80d890255ceef374c856318dde406a7aabca Mon Sep 17 00:00:00 2001 From: Duncan McGregor Date: Sun, 6 Jan 2013 17:34:38 +0000 Subject: [PATCH 2/2] #36 (JMOCK-256) Mocks being finalized report "the Mockery is not thread-safe: use a Synchroniser to ensure thread safety" Filter out finalizer invocations on cglib proxies Upgrate ClassImposteriserTests to JUnit4 --- .../jmock/lib/legacy/ClassImposteriser.java | 23 +++++-- .../lib/legacy/ClassImposteriserTests.java | 61 +++++++++++++++---- 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/src/org/jmock/lib/legacy/ClassImposteriser.java b/src/org/jmock/lib/legacy/ClassImposteriser.java index 2b04d0ea9..bdcfc9f9c 100644 --- a/src/org/jmock/lib/legacy/ClassImposteriser.java +++ b/src/org/jmock/lib/legacy/ClassImposteriser.java @@ -27,7 +27,9 @@ public class ClassImposteriser implements Imposteriser { public static final Imposteriser INSTANCE = new ClassImposteriser(); private ClassImposteriser() {} - + + private static final Method FINALIZE_METHOD = findFinalizeMethod(); + private static final NamingPolicy NAMING_POLICY_THAT_ALLOWS_IMPOSTERISATION_OF_CLASSES_IN_SIGNED_PACKAGES = new DefaultNamingPolicy() { @Override public String getClassName(String prefix, String source, Object key, Predicate names) { @@ -35,9 +37,14 @@ public String getClassName(String prefix, String source, Object key, Predicate n } }; - private static final CallbackFilter IGNORE_BRIDGE_METHODS = new CallbackFilter() { + private static final CallbackFilter IGNORED_METHODS = new CallbackFilter() { public int accept(Method method) { - return method.isBridge() ? 1 : 0; + if (method.isBridge()) + return 1; + else if (method.equals(FINALIZE_METHOD)) + return 1; + else + return 0; } }; @@ -105,7 +112,7 @@ protected void filterConstructors(Class sc, List constructors) { enhancer.setInterfaces(ancilliaryTypes); } enhancer.setCallbackTypes(new Class[]{InvocationHandler.class, NoOp.class}); - enhancer.setCallbackFilter(IGNORE_BRIDGE_METHODS); + enhancer.setCallbackFilter(IGNORED_METHODS); if (mockedType.getSigners() != null) { enhancer.setNamingPolicy(NAMING_POLICY_THAT_ALLOWS_IMPOSTERISATION_OF_CLASSES_IN_SIGNED_PACKAGES); } @@ -140,6 +147,14 @@ private Class[] prepend(Class first, Class... rest) { System.arraycopy(rest, 0, all, 1, rest.length); return all; } + + private static Method findFinalizeMethod() { + try { + return Object.class.getDeclaredMethod("finalize"); + } catch (NoSuchMethodException e) { + throw new IllegalStateException("Could not find finalize method on Object"); + } + } public static class ClassWithSuperclassToWorkAroundCglibBug {} } diff --git a/test/org/jmock/test/unit/lib/legacy/ClassImposteriserTests.java b/test/org/jmock/test/unit/lib/legacy/ClassImposteriserTests.java index 110c34a2a..b53276669 100644 --- a/test/org/jmock/test/unit/lib/legacy/ClassImposteriserTests.java +++ b/test/org/jmock/test/unit/lib/legacy/ClassImposteriserTests.java @@ -3,16 +3,25 @@ import junit.framework.TestCase; import org.jmock.api.Action; import org.jmock.api.Imposteriser; +import org.jmock.api.Invocation; +import org.jmock.api.Invokable; import org.jmock.lib.action.ReturnValueAction; import org.jmock.lib.action.VoidAction; import org.jmock.lib.legacy.ClassImposteriser; +import org.junit.Test; import java.io.File; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.net.URL; import java.net.URLClassLoader; import java.util.Date; -public class ClassImposteriserTests extends TestCase { +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class ClassImposteriserTests { Action action = new ReturnValueAction("result"); Imposteriser imposteriser = ClassImposteriser.INSTANCE; @@ -63,7 +72,8 @@ private AClassWithAPrivateConstructor(String someArgument) {} public String foo() {return "original result";} } - public void testCanImposteriseInterfacesAndNonFinalInstantiableClasses() { + @Test + public void canImposteriseInterfacesAndNonFinalInstantiableClasses() { assertTrue("should report that it can imposterise interfaces", imposteriser.canImposterise(Runnable.class)); assertTrue("should report that it can imposterise classes", @@ -82,28 +92,32 @@ public void testCanImposteriseInterfacesAndNonFinalInstantiableClasses() { !imposteriser.canImposterise(void.class)); } - public void testCanImposteriseAConcreteClassWithoutCallingItsConstructorOrInstanceInitialiserBlocks() { + @Test + public void canImposteriseAConcreteClassWithoutCallingItsConstructorOrInstanceInitialiserBlocks() { ConcreteClassWithNastyConstructor imposter = imposteriser.imposterise(action, ConcreteClassWithNastyConstructor.class); assertEquals("result", imposter.foo()); } - - public void testCanImposteriseAnInterface() { + + @Test + public void canImposteriseAnInterface() { AnInterface imposter = imposteriser.imposterise(action, AnInterface.class); assertEquals("result", imposter.foo()); } - - public void testCanImposteriseAClassWithAPrivateConstructor() { + + @Test + public void canImposteriseAClassWithAPrivateConstructor() { AClassWithAPrivateConstructor imposter = imposteriser.imposterise(action, AClassWithAPrivateConstructor.class); assertEquals("result", imposter.foo()); } - - public void testCanImposteriseAClassInASignedJarFile() throws Exception { + + @Test + public void canImposteriseAClassInASignedJarFile() throws Exception { File jarFile = new File("build/testdata/signed.jar"); assertTrue("Signed JAR file does not exist (use Ant to build it", jarFile.exists()); @@ -125,7 +139,8 @@ public final String toString() { } // See issue JMOCK-150 - public void testCannotImposteriseAClassWithAFinalToStringMethod() { + @Test + public void cannotImposteriseAClassWithAFinalToStringMethod() { assertTrue("should not be able to imposterise it", !imposteriser.canImposterise(ClassWithFinalToStringMethod.class)); try { @@ -136,15 +151,37 @@ public void testCannotImposteriseAClassWithAFinalToStringMethod() { } } - + + // See issue JMOCK-256 (Github #36) + @Test + public void doesntDelegateFinalizeMethod() throws Exception { + Invokable failIfInvokedAction = new Invokable() { + @Override + public Object invoke(Invocation invocation) throws Throwable { + fail("invocation should not have happened"); + return null; + } + }; + + Object imposter = imposteriser.imposterise(failIfInvokedAction, Object.class); + invokeMethod(imposter, Object.class.getDeclaredMethod("finalize")); + } + public interface EmptyInterface {} // See issue JMOCK-145 - public void testWorksAroundBugInCglibWhenAskedToImposteriseObject() { + @Test + public void worksAroundBugInCglibWhenAskedToImposteriseObject() { imposteriser.imposterise(new VoidAction(), Object.class); imposteriser.imposterise(new VoidAction(), Object.class, EmptyInterface.class); imposteriser.imposterise(new VoidAction(), Object.class, AnInterface.class); } + + private Object invokeMethod(Object object, Method method, Object... args) throws IllegalAccessException, InvocationTargetException { + method.setAccessible(true); + return method.invoke(object, args); + } + }