Polish proxy tests

spring-context/src/test/java/org/springframework/aop/framework/AbstractAopProxyTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,17 +28,13 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
-import junit.framework.TestCase;
 import org.aopalliance.aop.Advice;
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
+
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
-import test.mixin.LockMixin;
-import test.mixin.LockMixinAdvisor;
-import test.mixin.Lockable;
-import test.mixin.LockedException;
 
 import org.springframework.aop.Advisor;
 import org.springframework.aop.AfterReturningAdvice;
@@ -76,6 +72,11 @@ import org.springframework.tests.sample.beans.TestBean;
 import org.springframework.util.SerializationTestUtils;
 import org.springframework.util.StopWatch;
 
+import test.mixin.LockMixin;
+import test.mixin.LockMixinAdvisor;
+import test.mixin.Lockable;
+import test.mixin.LockedException;
+
 import static org.junit.Assert.*;
 
 /**
@@ -120,18 +121,12 @@ public abstract class AbstractAopProxyTests {
 	}
 
 
-	@Test
+	@Test(expected = AopConfigException.class)
 	public void testNoInterceptorsAndNoTarget() {
-		AdvisedSupport pc = new AdvisedSupport(new Class<?>[] {ITestBean.class});
+		AdvisedSupport pc = new AdvisedSupport(new Class<?>[] { ITestBean.class });
 		// Add no interceptors
-		try {
-			AopProxy aop = createAopProxy(pc);
-			aop.getProxy();
-			fail("Shouldn't allow no interceptors");
-		}
-		catch (AopConfigException ex) {
-			// Ok
-		}
+		AopProxy aop = createAopProxy(pc);
+		aop.getProxy();
 	}
 
 	/**
@@ -171,7 +166,6 @@ public abstract class AbstractAopProxyTests {
 		sw.start("Create " + howMany + " proxies");
 		testManyProxies(howMany);
 		sw.stop();
-		System.out.println(sw.getTotalTimeMillis());
 		assertTrue("Proxy creation was too slow",  sw.getTotalTimeMillis() < 5000);
 	}
 
@@ -243,7 +237,7 @@ public abstract class AbstractAopProxyTests {
 			p.echo(new IOException());
 		}
 		catch (IOException ex) {
-
+			/* expected */
 		}
 		assertEquals(1, cta.getCalls());
 
@@ -281,7 +275,6 @@ public abstract class AbstractAopProxyTests {
 
 		}
 		assertEquals(2, cta.getCalls());
-
 	}
 
 	/**
@@ -317,7 +310,6 @@ public abstract class AbstractAopProxyTests {
 		pf2.addAdvice(2, new CheckMethodInvocationIsSameInAndOutInterceptor());
 		pf2.addAdvice(1, new CheckMethodInvocationViaThreadLocalIsSameInAndOutInterceptor());
 		pf2.addAdvice(0, ExposeInvocationInterceptor.INSTANCE);
-		//System.err.println(pf2.toProxyConfigString());
 		ITestBean advised2 = (ITestBean) createProxy(pf2);
 		advised2.setAge(age2);
 		advised1.setSpouse(advised2); // = 2 invocations
@@ -379,20 +371,14 @@ public abstract class AbstractAopProxyTests {
 		assertEquals("3 more invocations via AOP as the first call was reentrant through the proxy", 4, di.getCount());
 	}
 
-
-	@Test
+	@Test(expected = IllegalStateException.class)
+	// Should fail to get proxy as exposeProxy wasn't set to true
 	public void testTargetCantGetProxyByDefault() {
 		NeedsToSeeProxy et = new NeedsToSeeProxy();
 		ProxyFactory pf1 = new ProxyFactory(et);
 		assertFalse(pf1.isExposeProxy());
 		INeedsToSeeProxy proxied = (INeedsToSeeProxy) createProxy(pf1);
-		try {
-			proxied.incrementViaProxy();
-			fail("Should have failed to get proxy as exposeProxy wasn't set to true");
-		}
-		catch (IllegalStateException ex) {
-			// Ok
-		}
+		proxied.incrementViaProxy();
 	}
 
 	@Test
@@ -417,7 +403,7 @@ public abstract class AbstractAopProxyTests {
 				if (!context) {
 					assertNoInvocationContext();
 				} else {
-					assertTrue("have context", ExposeInvocationInterceptor.currentInvocation() != null);
+					assertNotNull("have context", ExposeInvocationInterceptor.currentInvocation());
 				}
 				return s;
 			}
@@ -436,7 +422,7 @@ public abstract class AbstractAopProxyTests {
 		assertNoInvocationContext();
 		ITestBean tb = (ITestBean) aop.getProxy();
 		assertNoInvocationContext();
-		assertTrue("correct return value", tb.getName() == s);
+		assertSame("correct return value", s, tb.getName());
 	}
 
 	/**
@@ -453,7 +439,7 @@ public abstract class AbstractAopProxyTests {
 		pc.setTarget(raw);
 
 		ITestBean tb = (ITestBean) createProxy(pc);
-		assertTrue("this return is wrapped in proxy", tb.getSpouse() == tb);
+		assertSame("this return is wrapped in proxy", tb, tb.getSpouse());
 	}
 
 	@Test
@@ -593,15 +579,6 @@ public abstract class AbstractAopProxyTests {
 
 		ITestBean tb = (ITestBean) aop.getProxy();
 		tb.getName();
-		// Not safe to trap invocation
-		//assertTrue(tii.invocation == target.invocation);
-
-		//assertTrue(target.invocation.getProxy() == tb);
-
-	//	((IOther) tb).absquatulate();
-		//MethodInvocation minv =  tii.invocation;
-		//assertTrue("invoked on iother, not " + minv.getMethod().getDeclaringClass(), minv.getMethod().getDeclaringClass() == IOther.class);
-		//assertTrue(target.invocation == tii.invocation);
 	}
 
 	/**
@@ -645,13 +622,13 @@ public abstract class AbstractAopProxyTests {
 		int newAge = 65;
 		ITestBean itb = (ITestBean) createProxy(pc);
 		itb.setAge(newAge);
-		assertTrue(itb.getAge() == newAge);
+		assertEquals(newAge, itb.getAge());
 
 		Lockable lockable = (Lockable) itb;
 		assertFalse(lockable.locked());
 		lockable.lock();
 
-		assertTrue(itb.getAge() == newAge);
+		assertEquals(newAge, itb.getAge());
 		try {
 			itb.setAge(1);
 			fail("Setters should fail when locked");
@@ -659,16 +636,15 @@ public abstract class AbstractAopProxyTests {
 		catch (LockedException ex) {
 			// ok
 		}
-		assertTrue(itb.getAge() == newAge);
+		assertEquals(newAge, itb.getAge());
 
 		// Unlock
 		assertTrue(lockable.locked());
 		lockable.unlock();
 		itb.setAge(1);
-		assertTrue(itb.getAge() == 1);
+		assertEquals(1, itb.getAge());
 	}
 
-
 	@Test
 	public void testReplaceArgument() throws Throwable {
 		TestBean tb = new TestBean();
@@ -679,14 +655,14 @@ public abstract class AbstractAopProxyTests {
 		ITestBean t = (ITestBean) pc.getProxy();
 		int newAge = 5;
 		t.setAge(newAge);
-		assertTrue(t.getAge() == newAge);
+		assertEquals(newAge, t.getAge());
 		String newName = "greg";
 		t.setName(newName);
 		assertEquals(newName, t.getName());
 
 		t.setName(null);
 		// Null replacement magic should work
-		assertTrue(t.getName().equals(""));
+		assertEquals("", t.getName());
 	}
 
 	@Test
@@ -1310,9 +1286,9 @@ public abstract class AbstractAopProxyTests {
 		IOther proxyB = (IOther) createProxy(pfb);
 
 		assertEquals(pfa.getAdvisors().length, pfb.getAdvisors().length);
-		assertTrue(a.equals(b));
-		assertTrue(i1.equals(i2));
-		assertTrue(proxyA.equals(proxyB));
+		assertEquals(a, b);
+		assertEquals(i1, i2);
+		assertEquals(proxyA, proxyB);
 		assertEquals(proxyA.hashCode(), proxyB.hashCode());
 		assertFalse(proxyA.equals(a));
 
@@ -1682,9 +1658,7 @@ public abstract class AbstractAopProxyTests {
 		public Object invoke(MethodInvocation mi) throws Throwable {
 			Object proxy = AopContext.currentProxy();
 			Object ret = mi.proceed();
-			// TODO why does this cause stack overflow?
-			//assertEquals(proxy, AopContext.currentProxy());
-			assertTrue(proxy == AopContext.currentProxy());
+			assertEquals(proxy, AopContext.currentProxy());
 			return ret;
 		}
 	}
@@ -1785,7 +1759,7 @@ public abstract class AbstractAopProxyTests {
 	 * Note that trapping the Invocation as in previous version of this test
 	 * isn't safe, as invocations may be reused
 	 * and hence cleared at the end of each invocation.
-	 * So we trap only the targe.
+	 * So we trap only the target.
 	 */
 	protected static class TrapTargetInterceptor implements MethodInterceptor {
 
@@ -2105,11 +2079,10 @@ public abstract class AbstractAopProxyTests {
 	static class InvocationCheckExposedInvocationTestBean extends ExposedInvocationTestBean {
 		@Override
 		protected void assertions(MethodInvocation invocation) {
-			TestCase.assertTrue(invocation.getThis() == this);
-			TestCase.assertTrue("Invocation should be on ITestBean: " + invocation.getMethod(),
-					ITestBean.class.isAssignableFrom(invocation.getMethod().getDeclaringClass()));
+			assertSame(this, invocation.getThis());
+			assertTrue("Invocation should be on ITestBean: " + invocation.getMethod(),
+				ITestBean.class.isAssignableFrom(invocation.getMethod().getDeclaringClass()));
 		}
 	}
 
 }
-

spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java

@@ -21,6 +21,7 @@ import java.io.Serializable;
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
 import org.junit.Test;
+
 import test.mixin.LockMixinAdvisor;
 
 import org.springframework.aop.ClassFilter;
@@ -74,30 +75,17 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab
 		return true;
 	}
 
-
-	@Test
+	@Test(expected = IllegalArgumentException.class)
 	public void testNullConfig() {
-		try {
-			new CglibAopProxy(null);
-			fail("Shouldn't allow null interceptors");
-		}
-		catch (IllegalArgumentException ex) {
-			// Ok
-		}
+		new CglibAopProxy(null);
 	}
 
-	@Test
+	@Test(expected = AopConfigException.class)
 	public void testNoTarget() {
-		AdvisedSupport pc = new AdvisedSupport(new Class<?>[]{ITestBean.class});
+		AdvisedSupport pc = new AdvisedSupport(new Class<?>[] { ITestBean.class });
 		pc.addAdvice(new NopInterceptor());
-		try {
-			AopProxy aop = createAopProxy(pc);
-			aop.getProxy();
-			fail("Shouldn't allow no target with CGLIB proxy");
-		}
-		catch (AopConfigException ex) {
-			// Ok
-		}
+		AopProxy aop = createAopProxy(pc);
+		aop.getProxy();
 	}
 
 	@Test
@@ -210,7 +198,7 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab
 
 		ITestBean proxy1 = getAdvisedProxy(target);
 		ITestBean proxy2 = getAdvisedProxy(target2);
-		assertTrue(proxy1.getClass() == proxy2.getClass());
+		assertSame(proxy1.getClass(), proxy2.getClass());
 		assertEquals(target.getAge(), proxy1.getAge());
 		assertEquals(target2.getAge(), proxy2.getAge());
 	}
@@ -256,7 +244,7 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab
 
 		ITestBean proxy1 = getIntroductionAdvisorProxy(target);
 		ITestBean proxy2 = getIntroductionAdvisorProxy(target2);
-		assertTrue("Incorrect duplicate creation of proxy classes", proxy1.getClass() == proxy2.getClass());
+		assertSame("Incorrect duplicate creation of proxy classes", proxy1.getClass(), proxy2.getClass());
 	}
 
 	private ITestBean getIntroductionAdvisorProxy(TestBean target) {
@@ -427,6 +415,7 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab
 			return x + y;
 		}
 
+		@SuppressWarnings("unchecked")
 		public <V extends MyInterface> boolean doWithVarargs(V... args) {
 			return true;
 		}

spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java

@@ -20,6 +20,7 @@ import java.io.Serializable;
 
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
+
 import org.junit.Test;
 
 import org.springframework.aop.interceptor.ExposeInvocationInterceptor;
@@ -53,15 +54,9 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria
 	}
 
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
 	public void testNullConfig() {
-		try {
-			new JdkDynamicAopProxy(null);
-			fail("Shouldn't allow null interceptors");
-		}
-		catch (IllegalArgumentException ex) {
-			// Ok
-		}
+		new JdkDynamicAopProxy(null);
 	}
 
 	@Test
@@ -74,13 +69,13 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria
 
 		Object proxy = aop.getProxy();
 		assertTrue(proxy instanceof ITestBean);
-		assertTrue(!(proxy instanceof TestBean));
+		assertFalse(proxy instanceof TestBean);
 	}
 
 	@Test
 	public void testInterceptorIsInvokedWithNoTarget() throws Throwable {
 		// Test return value
-		final Integer age = 25;
+		final int age = 25;
 		MethodInterceptor mi = (invocation -> age);
 
 		AdvisedSupport pc = new AdvisedSupport(new Class<?>[] {ITestBean.class});
@@ -88,7 +83,7 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria
 		AopProxy aop = createAopProxy(pc);
 
 		ITestBean tb = (ITestBean) aop.getProxy();
-		assertTrue("correct return value", tb.getAge() == age);
+		assertEquals("correct return value", age, tb.getAge());
 	}
 
 	@Test
@@ -96,9 +91,9 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria
 		final ExposedInvocationTestBean expectedTarget = new ExposedInvocationTestBean() {
 			@Override
 			protected void assertions(MethodInvocation invocation) {
-				assertTrue(invocation.getThis() == this);
-				assertTrue("Invocation should be on ITestBean: " + invocation.getMethod(),
-						invocation.getMethod().getDeclaringClass() == ITestBean.class);
+				assertEquals(this, invocation.getThis());
+				assertEquals("Invocation should be on ITestBean: " + invocation.getMethod(),
+						ITestBean.class, invocation.getMethod().getDeclaringClass());
 			}
 		};
 
@@ -118,15 +113,6 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria
 
 		ITestBean tb = (ITestBean) aop.getProxy();
 		tb.getName();
-		// Not safe to trap invocation
-		//assertTrue(tii.invocation == target.invocation);
-
-		//assertTrue(target.invocation.getProxy() == tb);
-
-		//	((IOther) tb).absquatulate();
-		//MethodInvocation minv =  tii.invocation;
-		//assertTrue("invoked on iother, not " + minv.getMethod().getDeclaringClass(), minv.getMethod().getDeclaringClass() == IOther.class);
-		//assertTrue(target.invocation == tii.invocation);
 	}
 
 	@Test
@@ -139,7 +125,6 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria
 		Foo proxy = (Foo) createProxy(as);
 		assertSame("Target should be returned when return types are incompatible", bean, proxy.getBarThis());
 		assertSame("Proxy should be returned when return types are compatible", proxy, proxy.getFooThis());
-
 	}
 
 	@Test
@@ -149,8 +134,8 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria
 		JdkDynamicAopProxy aopProxy = new JdkDynamicAopProxy(as);
 		Named proxy = (Named) aopProxy.getProxy();
 		Named named = new Person();
-		assertEquals("equals() returned false", proxy, named);
-		assertEquals("hashCode() not equal", proxy.hashCode(), named.hashCode());
+		assertEquals("equals()", proxy, named);
+		assertEquals("hashCode()", proxy.hashCode(), named.hashCode());
 	}
 
 

spring-context/src/test/java/org/springframework/aop/framework/ObjenesisProxyTests.java

@@ -34,6 +34,7 @@ public class ObjenesisProxyTests {
 
 	@Test
 	public void appliesAspectToClassWithComplexConstructor() {
+		@SuppressWarnings("resource")
 		ApplicationContext context = new ClassPathXmlApplicationContext("ObjenesisProxyTests-context.xml", getClass());
 
 		ClassWithComplexConstructor bean = context.getBean(ClassWithComplexConstructor.class);