Merge pull request junit-team#293 from jglick/master

Sort test methods for predictability

build.xml

@@ -68,6 +68,7 @@
           debug="on"
           classpath="@{classpath}"
           includeantruntime="false"
+          source="1.5"
           target="1.5"
           >
         <compilerarg value="-Xlint:unchecked" />

src/main/java/junit/framework/TestSuite.java

@@ -10,6 +10,7 @@
 import java.util.Enumeration;
 import java.util.List;
 import java.util.Vector;
+import org.junit.internal.MethodSorter;
 
 /**
  * <p>A <code>TestSuite</code> is a <code>Composite</code> of Tests.
@@ -146,7 +147,7 @@ private void addTestsFromTestCase(final Class<?> theClass) {
 		Class<?> superClass= theClass;
 		List<String> names= new ArrayList<String>();
 		while (Test.class.isAssignableFrom(superClass)) {
-			for (Method each : superClass.getDeclaredMethods())
+			for (Method each : MethodSorter.getDeclaredMethods(superClass))
 				addTestMethod(each, names, theClass);
 			superClass= superClass.getSuperclass();
 		}

src/main/java/org/junit/internal/MethodSorter.java

@@ -0,0 +1,35 @@
+package org.junit.internal;
+
+import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.Comparator;
+
+public class MethodSorter {
+
+    /**
+     * Gets declared methods of a class in a predictable order.
+     * Using the JVM order is unwise since the Java platform does not
+     * specify any particular order, and in fact JDK 7 returns a more or less
+     * random order; well-written test code would not assume any order, but some
+     * does, and a predictable failure is better than a random failure on
+     * certain platforms. Uses an unspecified but deterministic order.
+     * @param clazz a class
+     * @return same as {@link Class#getDeclaredMethods} but sorted
+     * @see <a href="http://bugs.sun.com/view_bug.do?bug_id=7023180">JDK
+     *       (non-)bug #7023180</a>
+     */
+    public static Method[] getDeclaredMethods(Class<?> clazz) {
+        Method[] methods = clazz.getDeclaredMethods();
+        Arrays.sort(methods, new Comparator<Method>() {
+            @Override public int compare(Method m1, Method m2) {
+                int i1 = m1.getName().hashCode();
+                int i2 = m2.getName().hashCode();
+                return i1 != i2 ? i1 - i2 : m1.toString().compareTo(m2.toString());
+            }
+        });
+        return methods;
+    }
+
+    private MethodSorter() {}
+
+}

src/main/java/org/junit/internal/matchers/TypeSafeMatcher.java

@@ -3,6 +3,7 @@
 import java.lang.reflect.Method;
 
 import org.hamcrest.BaseMatcher;
+import org.junit.internal.MethodSorter;
 
 /**
  * Convenient base class for Matchers that require a non-null value of a specific type.
@@ -26,7 +27,7 @@ protected TypeSafeMatcher() {
 
     private static Class<?> findExpectedType(Class<?> fromClass) {
         for (Class<?> c = fromClass; c != Object.class; c = c.getSuperclass()) {
-            for (Method method : c.getDeclaredMethods()) {
+            for (Method method : MethodSorter.getDeclaredMethods(c)) {
                 if (isMatchesSafelyMethod(method)) {
                     return method.getParameterTypes()[0];
                 }

src/main/java/org/junit/internal/runners/TestClass.java

@@ -11,6 +11,7 @@
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.junit.internal.MethodSorter;
 import org.junit.runners.BlockJUnit4ClassRunner;
 
 /**
@@ -41,7 +42,7 @@ List<Method> getAfters() {
 	public List<Method> getAnnotatedMethods(Class<? extends Annotation> annotationClass) {
 		List<Method> results= new ArrayList<Method>();
 		for (Class<?> eachClass : getSuperClasses(fClass)) {
-			Method[] methods= eachClass.getDeclaredMethods();
+			Method[] methods= MethodSorter.getDeclaredMethods(eachClass);
 			for (Method eachMethod : methods) {
 				Annotation annotation= eachMethod.getAnnotation(annotationClass);
 				if (annotation != null && ! isShadowed(eachMethod, results)) 

src/main/java/org/junit/runners/model/TestClass.java

@@ -14,6 +14,7 @@
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
+import org.junit.internal.MethodSorter;
 
 /**
  * Wraps a class to be run, providing method validation and annotation searching
@@ -38,7 +39,7 @@ public TestClass(Class<?> klass) {
 					"Test class can only have one constructor");
 
 		for (Class<?> eachClass : getSuperClasses(fClass)) {
-			for (Method eachMethod : eachClass.getDeclaredMethods())
+			for (Method eachMethod : MethodSorter.getDeclaredMethods(eachClass))
 				addToAnnotationLists(new FrameworkMethod(eachMethod),
 						fMethodsForAnnotations);
 			for (Field eachField : eachClass.getDeclaredFields())

src/test/java/org/junit/internal/MethodSorterTest.java

@@ -0,0 +1,34 @@
+package org.junit.internal;
+
+import java.util.Arrays;
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+public class MethodSorterTest {
+
+    @Test public void getDeclaredMethods() throws Exception {
+        assertEquals("[void epsilon(), void beta(int[][]), java.lang.Object alpha(int,double,java.lang.Thread), void delta(), int gamma(), void gamma(boolean)]", declaredMethods(Dummy.class));
+        assertEquals("[void testOne()]", declaredMethods(Super.class));
+        assertEquals("[void testTwo()]", declaredMethods(Sub.class));
+    }
+
+    private static String declaredMethods(Class<?> c) {
+        return Arrays.toString(MethodSorter.getDeclaredMethods(c)).replace(c.getName() + '.', "");
+    }
+
+    private static class Dummy {
+        Object alpha(int i, double d, Thread t) {return null;}
+        void beta(int[][] x) {}
+        int gamma() {return 0;}
+        void gamma(boolean b) {}
+        void delta() {}
+        void epsilon() {}
+    }
+    private static class Super {
+        void testOne() {}
+    }
+    private static class Sub extends Super {
+        void testTwo() {}
+    }
+
+}

src/test/java/org/junit/tests/running/classes/ParentRunnerTest.java

@@ -33,7 +33,7 @@ public void apple() {
 		}
 
 		@Test
-		public void banana() {
+		public void /* must hash-sort after "apple" */Banana() {
 			log+= "banana ";
 		}
 	}