From f13f493551f6af6740a2aa6297b926b063ac6d44 Mon Sep 17 00:00:00 2001
From: Sam Brannen <sam@sambrannen.com>
Date: Sun, 17 May 2015 22:21:44 +0200
Subject: [PATCH] Introduce TestAnnotationUtils to reduce code duplication

---
 .../test/annotation/TestAnnotationUtils.java  | 66 +++++++++++++++++++
 .../junit4/SpringJUnit4ClassRunner.java       | 56 +++++-----------
 .../junit4/rules/SpringMethodRule.java        | 59 ++++-------------
 .../statements/SpringFailOnTimeout.java       | 22 +++++--
 .../junit4/statements/SpringRepeat.java       | 27 ++++++--
 5 files changed, 132 insertions(+), 98 deletions(-)
 create mode 100644 spring-test/src/main/java/org/springframework/test/annotation/TestAnnotationUtils.java

diff --git a/spring-test/src/main/java/org/springframework/test/annotation/TestAnnotationUtils.java b/spring-test/src/main/java/org/springframework/test/annotation/TestAnnotationUtils.java
new file mode 100644
index 0000000000..57cd5b3c0b
--- /dev/null
+++ b/spring-test/src/main/java/org/springframework/test/annotation/TestAnnotationUtils.java
@@ -0,0 +1,66 @@
+/*
+ * 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.test.annotation;
+
+import java.lang.reflect.Method;
+
+import org.springframework.core.annotation.AnnotatedElementUtils;
+import org.springframework.core.annotation.AnnotationAttributes;
+import org.springframework.core.annotation.AnnotationUtils;
+
+/**
+ * Collection of utility methods for working with Spring's core testing annotations.
+ *
+ * @author Sam Brannen
+ * @since 4.2
+ */
+public class TestAnnotationUtils {
+
+	/**
+	 * Get the {@code timeout} configured via the {@link Timed @Timed}
+	 * annotation on the supplied {@code method}.
+	 * <p>Negative configured values will be converted to {@code 0}.
+	 * @return the configured timeout, or {@code 0} if the method is not
+	 * annotated with {@code @Timed}
+	 */
+	public static long getTimeout(Method method) {
+		AnnotationAttributes attributes = AnnotatedElementUtils.findAnnotationAttributes(method, Timed.class.getName());
+		if (attributes == null) {
+			return 0;
+		}
+		else {
+			long millis = attributes.<Long> getNumber("millis").longValue();
+			return Math.max(0, millis);
+		}
+	}
+
+	/**
+	 * Get the repeat count configured via the {@link Repeat @Repeat}
+	 * annotation on the supplied {@code method}.
+	 * <p>Non-negative configured values will be converted to {@code 1}.
+	 * @return the configured repeat count, or {@code 1} if the method is
+	 * not annotated with {@code @Repeat}
+	 */
+	public static int getRepeatCount(Method method) {
+		Repeat repeat = AnnotationUtils.findAnnotation(method, Repeat.class);
+		if (repeat == null) {
+			return 1;
+		}
+		return Math.max(1, repeat.value());
+	}
+
+}
diff --git a/spring-test/src/main/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunner.java b/spring-test/src/main/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunner.java
index 6762e37ca6..45a4d13d98 100644
--- a/spring-test/src/main/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunner.java
+++ b/spring-test/src/main/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunner.java
@@ -35,12 +35,8 @@ import org.junit.runners.model.FrameworkMethod;
 import org.junit.runners.model.InitializationError;
 import org.junit.runners.model.Statement;
 
-import org.springframework.core.annotation.AnnotatedElementUtils;
-import org.springframework.core.annotation.AnnotationAttributes;
-import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.test.annotation.ProfileValueUtils;
-import org.springframework.test.annotation.Repeat;
-import org.springframework.test.annotation.Timed;
+import org.springframework.test.annotation.TestAnnotationUtils;
 import org.springframework.test.context.TestContextManager;
 import org.springframework.test.context.junit4.rules.SpringClassRule;
 import org.springframework.test.context.junit4.rules.SpringMethodRule;
@@ -68,8 +64,8 @@ import org.springframework.util.ReflectionUtils;
  * <ul>
  * <li>{@link Test#expected() @Test(expected=...)}</li>
  * <li>{@link Test#timeout() @Test(timeout=...)}</li>
- * <li>{@link Timed @Timed}</li>
- * <li>{@link Repeat @Repeat}</li>
+ * <li>{@link org.springframework.test.annotation.Timed @Timed}</li>
+ * <li>{@link org.springframework.test.annotation.Repeat @Repeat}</li>
  * <li>{@link Ignore @Ignore}</li>
  * <li>{@link org.springframework.test.annotation.ProfileValueSourceConfiguration @ProfileValueSourceConfiguration}</li>
  * <li>{@link org.springframework.test.annotation.IfProfileValue @IfProfileValue}</li>
@@ -338,19 +334,16 @@ public class SpringJUnit4ClassRunner extends BlockJUnit4ClassRunner {
 	 * @return the expected exception, or {@code null} if none was specified
 	 */
 	protected Class<? extends Throwable> getExpectedException(FrameworkMethod frameworkMethod) {
-		Test testAnnotation = frameworkMethod.getAnnotation(Test.class);
-		Class<? extends Throwable> junitExpectedException = (testAnnotation != null
-				&& testAnnotation.expected() != Test.None.class ? testAnnotation.expected() : null);
-
-		return junitExpectedException;
+		Test test = frameworkMethod.getAnnotation(Test.class);
+		return ((test != null) && (test.expected() != Test.None.class) ? test.expected() : null);
 	}
 
 	/**
 	 * Perform the same logic as
 	 * {@link BlockJUnit4ClassRunner#withPotentialTimeout(FrameworkMethod, Object, Statement)}
 	 * but with additional support for Spring's {@code @Timed} annotation.
-	 * <p>Supports both Spring's {@link Timed @Timed} and JUnit's
-	 * {@link Test#timeout() @Test(timeout=...)} annotations, but not both
+	 * <p>Supports both Spring's {@link org.springframework.test.annotation.Timed @Timed}
+	 * and JUnit's {@link Test#timeout() @Test(timeout=...)} annotations, but not both
 	 * simultaneously.
 	 * @return either a {@link SpringFailOnTimeout}, a {@link FailOnTimeout},
 	 * or the supplied {@link Statement} as appropriate
@@ -391,26 +384,19 @@ public class SpringJUnit4ClassRunner extends BlockJUnit4ClassRunner {
 	 * @return the timeout, or {@code 0} if none was specified
 	 */
 	protected long getJUnitTimeout(FrameworkMethod frameworkMethod) {
-		Test testAnnotation = frameworkMethod.getAnnotation(Test.class);
-		return (testAnnotation != null && testAnnotation.timeout() > 0 ? testAnnotation.timeout() : 0);
+		Test test = frameworkMethod.getAnnotation(Test.class);
+		return ((test != null) && (test.timeout() > 0) ? test.timeout() : 0);
 	}
 
 	/**
 	 * Retrieve the configured Spring-specific {@code timeout} from the
-	 * {@link Timed @Timed} annotation on the supplied
-	 * {@linkplain FrameworkMethod test method}.
+	 * {@link org.springframework.test.annotation.Timed @Timed} annotation
+	 * on the supplied {@linkplain FrameworkMethod test method}.
 	 * @return the timeout, or {@code 0} if none was specified
+	 * @see TestAnnotationUtils#getTimeout(Method)
 	 */
 	protected long getSpringTimeout(FrameworkMethod frameworkMethod) {
-		AnnotationAttributes annAttrs = AnnotatedElementUtils.findAnnotationAttributes(frameworkMethod.getMethod(),
-			Timed.class.getName());
-		if (annAttrs == null) {
-			return 0;
-		}
-		else {
-			long millis = annAttrs.<Long> getNumber("millis").longValue();
-			return millis > 0 ? millis : 0;
-		}
+		return TestAnnotationUtils.getTimeout(frameworkMethod.getMethod());
 	}
 
 	/**
@@ -442,20 +428,14 @@ public class SpringJUnit4ClassRunner extends BlockJUnit4ClassRunner {
 	}
 
 	/**
-	 * Return a {@link Statement} that potentially repeats the execution of
-	 * the {@code next} statement.
-	 * <p>Supports Spring's {@link Repeat @Repeat} annotation by returning a
-	 * {@code SpringRepeat} statement initialized with the configured repeat
-	 * count (if greater than {@code 1}); otherwise, the supplied statement
-	 * is returned unmodified.
-	 * @return either a {@code SpringRepeat} or the supplied {@code Statement}
-	 * as appropriate
+	 * Wrap the supplied {@link Statement} with a {@code SpringRepeat} statement.
+	 * <p>Supports Spring's {@link org.springframework.test.annotation.Repeat @Repeat}
+	 * annotation.
+	 * @see TestAnnotationUtils#getRepeatCount(Method)
 	 * @see SpringRepeat
 	 */
 	protected Statement withPotentialRepeat(FrameworkMethod frameworkMethod, Object testInstance, Statement next) {
-		Repeat repeatAnnotation = AnnotationUtils.getAnnotation(frameworkMethod.getMethod(), Repeat.class);
-		int repeat = (repeatAnnotation != null ? repeatAnnotation.value() : 1);
-		return (repeat > 1 ? new SpringRepeat(next, frameworkMethod.getMethod(), repeat) : next);
+		return new SpringRepeat(next, frameworkMethod.getMethod());
 	}
 
 }
diff --git a/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringMethodRule.java b/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringMethodRule.java
index 5c0255eabe..8772387828 100644
--- a/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringMethodRule.java
+++ b/spring-test/src/main/java/org/springframework/test/context/junit4/rules/SpringMethodRule.java
@@ -26,11 +26,7 @@ import org.junit.rules.MethodRule;
 import org.junit.runners.model.FrameworkMethod;
 import org.junit.runners.model.Statement;
 
-import org.springframework.core.annotation.AnnotatedElementUtils;
-import org.springframework.core.annotation.AnnotationAttributes;
-import org.springframework.core.annotation.AnnotationUtils;
-import org.springframework.test.annotation.Repeat;
-import org.springframework.test.annotation.Timed;
+import org.springframework.test.annotation.TestAnnotationUtils;
 import org.springframework.test.context.TestContextManager;
 import org.springframework.test.context.junit4.statements.ProfileValueChecker;
 import org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks;
@@ -78,8 +74,8 @@ import org.springframework.util.ReflectionUtils;
  * implementations.)</em>
  *
  * <ul>
- * <li>{@link Timed @Timed}</li>
- * <li>{@link Repeat @Repeat}</li>
+ * <li>{@link org.springframework.test.annotation.Timed @Timed}</li>
+ * <li>{@link org.springframework.test.annotation.Repeat @Repeat}</li>
  * <li>{@link org.springframework.test.annotation.ProfileValueSourceConfiguration @ProfileValueSourceConfiguration}</li>
  * <li>{@link org.springframework.test.annotation.IfProfileValue @IfProfileValue}</li>
  * </ul>
@@ -180,54 +176,25 @@ public class SpringMethodRule implements MethodRule {
 	}
 
 	/**
-	 * Return a {@link Statement} that potentially repeats the execution of
-	 * the {@code next} statement.
-	 * <p>Supports Spring's {@link Repeat @Repeat} annotation by returning a
-	 * {@link SpringRepeat} statement initialized with the configured repeat
-	 * count (if greater than {@code 1}); otherwise, the supplied statement
-	 * is returned unmodified.
-	 * @return either a {@code SpringRepeat} or the supplied {@code Statement}
+	 * Wrap the supplied {@link Statement} with a {@code SpringRepeat} statement.
+	 * <p>Supports Spring's {@link org.springframework.test.annotation.Repeat @Repeat}
+	 * annotation.
+	 * @see TestAnnotationUtils#getRepeatCount(java.lang.reflect.Method)
 	 * @see SpringRepeat
 	 */
 	private Statement withPotentialRepeat(Statement next, FrameworkMethod frameworkMethod, Object testInstance) {
-		Repeat repeatAnnotation = AnnotationUtils.getAnnotation(frameworkMethod.getMethod(), Repeat.class);
-		int repeat = (repeatAnnotation != null ? repeatAnnotation.value() : 1);
-		return (repeat > 1 ? new SpringRepeat(next, frameworkMethod.getMethod(), repeat) : next);
+		return new SpringRepeat(next, frameworkMethod.getMethod());
 	}
 
 	/**
-	 * Return a {@link Statement} that potentially throws an exception if
-	 * the {@code next} statement in the execution chain takes longer than
-	 * a specified timeout.
-	 * <p>Supports Spring's {@link Timed @Timed} annotation by returning a
-	 * {@link SpringFailOnTimeout} statement initialized with the configured
-	 * timeout (if greater than {@code 0}); otherwise, the supplied statement
-	 * is returned unmodified.
-	 * @return either a {@code SpringFailOnTimeout} or the supplied {@code Statement}
-	 * @see #getSpringTimeout(FrameworkMethod)
+	 * Wrap the supplied {@link Statement} with a {@code SpringFailOnTimeout} statement.
+	 * <p>Supports Spring's {@link org.springframework.test.annotation.Timed @Timed}
+	 * annotation.
+	 * @see TestAnnotationUtils#getTimeout(java.lang.reflect.Method)
 	 * @see SpringFailOnTimeout
 	 */
 	private Statement withPotentialTimeout(Statement next, FrameworkMethod frameworkMethod, Object testInstance) {
-		long springTimeout = getSpringTimeout(frameworkMethod);
-		return (springTimeout > 0 ? new SpringFailOnTimeout(next, springTimeout) : next);
-	}
-
-	/**
-	 * Retrieve the configured Spring-specific {@code timeout} from the
-	 * {@link Timed @Timed} annotation on the supplied
-	 * {@linkplain FrameworkMethod test method}.
-	 * @return the timeout, or {@code 0} if none was specified
-	 */
-	private long getSpringTimeout(FrameworkMethod frameworkMethod) {
-		AnnotationAttributes annAttrs = AnnotatedElementUtils.findAnnotationAttributes(frameworkMethod.getMethod(),
-			Timed.class.getName());
-		if (annAttrs == null) {
-			return 0;
-		}
-		else {
-			long millis = annAttrs.<Long> getNumber("millis").longValue();
-			return millis > 0 ? millis : 0;
-		}
+		return new SpringFailOnTimeout(next, TestAnnotationUtils.getTimeout(frameworkMethod.getMethod()));
 	}
 
 	/**
diff --git a/spring-test/src/main/java/org/springframework/test/context/junit4/statements/SpringFailOnTimeout.java b/spring-test/src/main/java/org/springframework/test/context/junit4/statements/SpringFailOnTimeout.java
index b12646ab28..d451d21c54 100644
--- a/spring-test/src/main/java/org/springframework/test/context/junit4/statements/SpringFailOnTimeout.java
+++ b/spring-test/src/main/java/org/springframework/test/context/junit4/statements/SpringFailOnTimeout.java
@@ -19,8 +19,8 @@ package org.springframework.test.context.junit4.statements;
 import java.util.concurrent.TimeoutException;
 
 import org.junit.runners.model.Statement;
-
 import org.springframework.test.annotation.Timed;
+import org.springframework.util.Assert;
 
 /**
  * {@code SpringFailOnTimeout} is a custom JUnit {@link Statement} which adds
@@ -52,6 +52,8 @@ public class SpringFailOnTimeout extends Statement {
 	 * @see Timed#millis()
 	 */
 	public SpringFailOnTimeout(Statement next, long timeout) {
+		Assert.notNull(next, "next statement must not be null");
+		Assert.isTrue(timeout >= 0, "timeout must be non-negative");
 		this.next = next;
 		this.timeout = timeout;
 	}
@@ -64,14 +66,20 @@ public class SpringFailOnTimeout extends Statement {
 	 */
 	@Override
 	public void evaluate() throws Throwable {
-		long startTime = System.currentTimeMillis();
-		try {
+		if (this.timeout == 0) {
 			this.next.evaluate();
 		}
-		finally {
-			long elapsed = System.currentTimeMillis() - startTime;
-			if (elapsed > this.timeout) {
-				throw new TimeoutException(String.format("Test took %s ms; limit was %s ms.", elapsed, this.timeout));
+		else {
+			long startTime = System.currentTimeMillis();
+			try {
+				this.next.evaluate();
+			}
+			finally {
+				long elapsed = System.currentTimeMillis() - startTime;
+				if (elapsed > this.timeout) {
+					throw new TimeoutException(
+						String.format("Test took %s ms; limit was %s ms.", elapsed, this.timeout));
+				}
 			}
 		}
 	}
diff --git a/spring-test/src/main/java/org/springframework/test/context/junit4/statements/SpringRepeat.java b/spring-test/src/main/java/org/springframework/test/context/junit4/statements/SpringRepeat.java
index 6b685012a0..b00833a520 100644
--- a/spring-test/src/main/java/org/springframework/test/context/junit4/statements/SpringRepeat.java
+++ b/spring-test/src/main/java/org/springframework/test/context/junit4/statements/SpringRepeat.java
@@ -20,15 +20,15 @@ import java.lang.reflect.Method;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.junit.runners.model.Statement;
 
-import org.springframework.test.annotation.Repeat;
-import org.springframework.util.ClassUtils;
+import org.springframework.test.annotation.TestAnnotationUtils;
 
 /**
  * {@code SpringRepeat} is a custom JUnit {@link Statement} which adds support
- * for Spring's {@link Repeat @Repeat} annotation by repeating the test the
- * specified number of times.
+ * for Spring's {@link org.springframework.test.annotation.Repeat @Repeat}
+ * annotation by repeating the test the specified number of times.
  *
  * @see #evaluate()
  * @author Sam Brannen
@@ -46,12 +46,25 @@ public class SpringRepeat extends Statement {
 
 
 	/**
-	 * Construct a new {@code SpringRepeat} statement.
+	 * Construct a new {@code SpringRepeat} statement for the supplied
+	 * {@code testMethod}, retrieving the configured repeat count from the
+	 * {@code @Repeat} annotation on the supplied method.
+	 *
+	 * @param next the next {@code Statement} in the execution chain
+	 * @param testMethod the current test method
+	 * @see TestAnnotationUtils#getRepeatCount(Method)
+	 */
+	public SpringRepeat(Statement next, Method testMethod) {
+		this(next, testMethod, TestAnnotationUtils.getRepeatCount(testMethod));
+	}
+
+	/**
+	 * Construct a new {@code SpringRepeat} statement for the supplied
+	 * {@code testMethod} and {@code repeat} count.
 	 *
 	 * @param next the next {@code Statement} in the execution chain
 	 * @param testMethod the current test method
 	 * @param repeat the configured repeat count for the current test method
-	 * @see Repeat#value()
 	 */
 	public SpringRepeat(Statement next, Method testMethod, int repeat) {
 		this.next = next;
@@ -68,7 +81,7 @@ public class SpringRepeat extends Statement {
 		for (int i = 0; i < this.repeat; i++) {
 			if (this.repeat > 1 && logger.isInfoEnabled()) {
 				logger.info(String.format("Repetition %d of test %s#%s()", (i + 1),
-					ClassUtils.getShortName(this.testMethod.getDeclaringClass()), this.testMethod.getName()));
+					this.testMethod.getDeclaringClass().getSimpleName(), this.testMethod.getName()));
 			}
 			this.next.evaluate();
 		}