Never reorder classes annotated with @FixMethodOrder

Fixes junit-team#1637

src/main/java/org/junit/runner/OrderWith.java

@@ -7,6 +7,7 @@
 import java.lang.annotation.Target;
 
 import org.junit.runner.manipulation.Ordering;
+import org.junit.validator.ValidateWith;
 
 /**
  * When a test class is annotated with <code>&#064;OrderWith</code> or extends a class annotated
@@ -18,6 +19,7 @@
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.TYPE)
 @Inherited
+@ValidateWith(OrderWithValidator.class)
 public @interface OrderWith {
     /**
      * Gets a class that extends {@link Ordering}. The class must have a public no-arg constructor.

src/main/java/org/junit/runner/OrderWithValidator.java

@@ -0,0 +1,38 @@
+package org.junit.runner;
+
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
+
+import java.util.List;
+
+import org.junit.FixMethodOrder;
+import org.junit.runners.model.TestClass;
+import org.junit.validator.AnnotationValidator;
+
+/**
+ * Validates that there are no errors in the use of the {@code OrderWith}
+ * annotation. If there is, a {@code Throwable} object will be added to the list
+ * of errors.
+ *
+ * @since 4.13
+ */
+public final class OrderWithValidator extends AnnotationValidator {
+
+    /**
+     * Adds to {@code errors} a throwable for each problem detected. Looks for
+     * {@code FixMethodOrder} annotations.
+     *
+     * @param testClass that is being validated
+     * @return A list of exceptions detected
+     *
+     * @since 4.13
+     */
+    @Override
+    public List<Exception> validateAnnotatedClass(TestClass testClass) {
+        if (testClass.getAnnotation(FixMethodOrder.class) != null) {
+            return singletonList(
+                    new Exception("@FixMethodOrder cannot be combined with @OrderWith"));
+        }
+        return emptyList();
+    }
+}

src/main/java/org/junit/runners/ParentRunner.java

@@ -19,6 +19,7 @@
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
+import org.junit.FixMethodOrder;
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.internal.AssumptionViolatedException;
@@ -451,6 +452,10 @@ public void filter(Filter filter) throws NoTestsRemainException {
     }
 
     public void sort(Sorter sorter) {
+        if (shouldNotReorder()) {
+            return;
+        }
+
         childrenLock.lock();
         try {
             for (T each : getFilteredChildren()) {
@@ -470,6 +475,10 @@ public void sort(Sorter sorter) {
      * @since 4.13
      */
     public void order(Orderer orderer) throws InvalidOrderingException {
+        if (shouldNotReorder()) {
+            return;
+        }
+
         childrenLock.lock();
         try {
             List<T> children = getFilteredChildren();
@@ -504,6 +513,11 @@ public void order(Orderer orderer) throws InvalidOrderingException {
     // Private implementation
     //
 
+    private boolean shouldNotReorder() {
+        // If the test specifies a specific order, do not reorder.
+        return getDescription().getAnnotation(FixMethodOrder.class) != null;
+    }
+
     private void validate() throws InitializationError {
         List<Throwable> errors = new ArrayList<Throwable>();
         collectInitializationErrors(errors);

src/test/java/org/junit/tests/manipulation/OrderableTest.java

@@ -3,6 +3,7 @@
 import static org.junit.Assert.assertEquals;
 import junit.framework.JUnit4TestAdapter;
 import org.junit.Before;
+import org.junit.FixMethodOrder;
 import org.junit.Test;
 import org.junit.experimental.runners.Enclosed;
 import org.junit.runner.Description;
@@ -16,6 +17,7 @@
 import org.junit.runner.manipulation.Sorter;
 import org.junit.runner.notification.RunNotifier;
 import org.junit.runners.BlockJUnit4ClassRunner;
+import org.junit.runners.MethodSorters;
 
 @RunWith(Enclosed.class)
 public class OrderableTest {
@@ -40,6 +42,24 @@ public void c() {
             }
         }
 
+        @FixMethodOrder(MethodSorters.NAME_ASCENDING)
+        public static class DoNotOrderMe {
+            @Test
+            public void a() {
+                log += "a";
+            }
+
+            @Test
+            public void b() {
+                log += "b";
+            }
+
+            @Test
+            public void c() {
+                log += "c";
+            }
+        }
+
         @Before
         public void resetLog() {
             log = "";
@@ -62,6 +82,15 @@ public void orderingBackwardWorksOnTestClassRunner() {
             new JUnitCore().run(backward);
             assertEquals("cba", log);
         }
+
+        @Test
+        public void orderingBackwardDoesNothingOnTestClassRunnerWithFixMethodOrder() {
+            Request backward = Request.aClass(DoNotOrderMe.class).orderWith(
+                    new ReverseAlphanumericOrdering());
+
+            new JUnitCore().run(backward);
+            assertEquals("abc", log);
+        }
 
         @RunWith(Enclosed.class)
         public static class Enclosing {

src/test/java/org/junit/tests/manipulation/SortableTest.java

@@ -7,6 +7,7 @@
 
 import junit.framework.JUnit4TestAdapter;
 import org.junit.Before;
+import org.junit.FixMethodOrder;
 import org.junit.Test;
 import org.junit.experimental.runners.Enclosed;
 import org.junit.runner.Description;
@@ -19,6 +20,7 @@
 import org.junit.runner.manipulation.Sorter;
 import org.junit.runner.notification.RunNotifier;
 import org.junit.runners.BlockJUnit4ClassRunner;
+import org.junit.runners.MethodSorters;
 
 @RunWith(Enclosed.class)
 public class SortableTest {
@@ -50,6 +52,24 @@ public void c() {
             }
         }
 
+        @FixMethodOrder(MethodSorters.NAME_ASCENDING)
+        public static class DoNotSortMe {
+            @Test
+            public void a() {
+                log += "a";
+            }
+
+            @Test
+            public void b() {
+                log += "b";
+            }
+
+            @Test
+            public void c() {
+                log += "c";
+            }
+        }
+
         @Before
         public void resetLog() {
             log = "";
@@ -71,6 +91,14 @@ public void sortingBackwardWorksOnTestClassRunner() {
             assertEquals("cba", log);
         }
 
+        @Test
+        public void sortingBackwardDoesNothingOnTestClassRunnerWithFixMethodOrder() {
+            Request backward = Request.aClass(DoNotSortMe.class).sortWith(backward());
+
+            new JUnitCore().run(backward);
+            assertEquals("abc", log);
+        }
+
         @RunWith(Enclosed.class)
         public static class Enclosing {
             public static class A {