Recommend order attribute for TestWatcher and tidy up tests

src/main/java/org/junit/rules/TestWatcher.java

@@ -4,6 +4,7 @@
 import java.util.List;
 
 import org.junit.AssumptionViolatedException;
+import org.junit.Rule;
 import org.junit.runner.Description;
 import org.junit.runners.model.MultipleFailureException;
 import org.junit.runners.model.Statement;
@@ -17,7 +18,7 @@
  * public static class WatchmanTest {
  *  private static String watchedLog;
  *
- *  @Rule
+ *  @Rule(order = Integer.MIN_VALUE)
  *  public TestWatcher watchman= new TestWatcher() {
  *      @Override
  *      protected void failed(Throwable e, Description description) {
@@ -40,6 +41,11 @@
  *     }
  * }
  * </pre>
+ * <p>It is recommended to always set the {@link Rule#order() order} of the
+ * {@code TestWatcher} to {@code Integer.MIN_VALUE} so that it encloses all
+ * other rules. Otherwise it may see failed tests as successful and vice versa
+ * if some rule changes the result of a test (e.g. {@link ErrorCollector} or
+ * {@link ExpectedException}).
  *
  * @since 4.9
  */

src/test/java/org/junit/rules/LoggingTestWatcher.java

@@ -25,6 +25,11 @@ protected void skipped(AssumptionViolatedException e, Description description) {
         log.append("skipped ");
     }
 
+    @Override
+    protected void skipped(org.junit.internal.AssumptionViolatedException e, Description description) {
+        log.append("deprecated skipped ");
+    }
+
     @Override
     protected void starting(Description description) {
         log.append("starting ");

src/test/java/org/junit/rules/TestRuleTest.java

@@ -148,64 +148,6 @@ public void ignoreNonRules() {
 
     private static String log;
 
-    public static class OnFailureTest {
-        @Rule
-        public TestRule watcher = new TestWatcher() {
-            @Override
-            protected void failed(Throwable e, Description description) {
-                log += description + " " + e.getClass().getSimpleName();
-            }
-        };
-
-        @Test
-        public void nothing() {
-            fail();
-        }
-    }
-
-    @Test
-    public void onFailure() {
-        log = "";
-        Result result = JUnitCore.runClasses(OnFailureTest.class);
-        assertEquals(String.format("nothing(%s) AssertionError", OnFailureTest.class.getName()), log);
-        assertEquals(1, result.getFailureCount());
-    }
-
-    public static class WatchmanTest {
-        private static String watchedLog;
-
-        @Rule
-        public TestRule watcher = new TestWatcher() {
-            @Override
-            protected void failed(Throwable e, Description description) {
-                watchedLog += description + " "
-                        + e.getClass().getSimpleName() + "\n";
-            }
-
-            @Override
-            protected void succeeded(Description description) {
-                watchedLog += description + " " + "success!\n";
-            }
-        };
-
-        @Test
-        public void fails() {
-            fail();
-        }
-
-        @Test
-        public void succeeds() {
-        }
-    }
-
-    @Test
-    public void succeeded() {
-        WatchmanTest.watchedLog = "";
-        JUnitCore.runClasses(WatchmanTest.class);
-        assertThat(WatchmanTest.watchedLog, containsString(String.format("fails(%s) AssertionError", WatchmanTest.class.getName())));
-        assertThat(WatchmanTest.watchedLog, containsString(String.format("succeeds(%s) success!", WatchmanTest.class.getName())));
-    }
-
     public static class BeforesAndAfters {
         private static StringBuilder watchedLog = new StringBuilder();
 
@@ -437,101 +379,6 @@ public void methodIgnoreNonRules() {
         assertEquals(0, result.getFailureCount());
     }
 
-    public static class MethodOnFailureTest {
-        private TestRule watchman = new TestWatcher() {
-            @Override
-            protected void failed(Throwable e, Description description) {
-                log += description + " " + e.getClass().getSimpleName();
-            }
-        };
-
-        @Rule
-        public TestRule getWatchman() {
-            return watchman;
-        }
-
-        @Test
-        public void nothing() {
-            fail();
-        }
-    }
-
-    @Test
-    public void methodOnFailure() {
-        log = "";
-        Result result = JUnitCore.runClasses(MethodOnFailureTest.class);
-        assertEquals(String.format("nothing(%s) AssertionError", MethodOnFailureTest.class.getName()), log);
-        assertEquals(1, result.getFailureCount());
-    }
-
-    public static class MethodOnSkippedTest {
-        private TestRule watchman = new TestWatcher() {
-            @Override
-            protected void skipped(AssumptionViolatedException e, Description description) {
-                log += description + " " + e.getClass().getSimpleName();
-            }
-        };
-
-        @Rule
-        public TestRule getWatchman() {
-            return watchman;
-        }
-
-        @Test
-        public void nothing() {
-            Assume.assumeTrue(false);
-        }
-    }
-
-    @Test
-    public void methodOnSkipped() {
-        log = "";
-        Result result = JUnitCore.runClasses(MethodOnSkippedTest.class);
-        assertEquals(String.format("nothing(%s) AssumptionViolatedException", MethodOnSkippedTest.class.getName()), log);
-        assertEquals(0, result.getFailureCount());
-        assertEquals(1, result.getRunCount());
-    }
-
-    public static class MethodWatchmanTest {
-        @SuppressWarnings("unused")
-        private static String watchedLog;
-
-        private TestRule watchman = new TestWatcher() {
-            @Override
-            protected void failed(Throwable e, Description description) {
-                watchedLog += description + " "
-                        + e.getClass().getSimpleName() + "\n";
-            }
-
-            @Override
-            protected void succeeded(Description description) {
-                watchedLog += description + " " + "success!\n";
-            }
-        };
-
-        @Rule
-        public TestRule getWatchman() {
-            return watchman;
-        }
-
-        @Test
-        public void fails() {
-            fail();
-        }
-
-        @Test
-        public void succeeds() {
-        }
-    }
-
-    @Test
-    public void methodSucceeded() {
-        WatchmanTest.watchedLog = "";
-        JUnitCore.runClasses(WatchmanTest.class);
-        assertThat(WatchmanTest.watchedLog, containsString(String.format("fails(%s) AssertionError", WatchmanTest.class.getName())));
-        assertThat(WatchmanTest.watchedLog, containsString(String.format("succeeds(%s) success!", WatchmanTest.class.getName())));
-    }
-
     public static class BeforesAndAftersAreEnclosedByRule {
         private static StringBuilder log;