Resolved Issue#589 where the method rules returned my methods were not executed

Author: npathai

Diff for: src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java

@@ -20,6 +20,7 @@
 import org.junit.internal.runners.statements.InvokeMethod;
 import org.junit.internal.runners.statements.RunAfters;
 import org.junit.internal.runners.statements.RunBefores;
+import org.junit.rules.MethodRule;
 import org.junit.rules.RunRules;
 import org.junit.rules.TestRule;
 import org.junit.runner.Description;
@@ -376,9 +377,14 @@ private List<org.junit.rules.MethodRule> getMethodRules(Object target) {
      * @return a list of MethodRules that should be applied when executing this
      *         test
      */
-    protected List<org.junit.rules.MethodRule> rules(Object target) {
-        return getTestClass().getAnnotatedFieldValues(target, Rule.class,
-                org.junit.rules.MethodRule.class);
+    protected List<MethodRule> rules(Object target) {
+        List<MethodRule> rules = getTestClass().getAnnotatedMethodValues(target, 
+                Rule.class, MethodRule.class);
+        
+        rules.addAll(getTestClass().getAnnotatedFieldValues(target,
+                Rule.class, MethodRule.class));
+        
+        return rules;
     }
 
     /**

Diff for: src/main/java/org/junit/runners/model/TestClass.java

@@ -244,8 +244,16 @@ public <T> List<T> getAnnotatedMethodValues(Object test,
         List<T> results = new ArrayList<T>();
         for (FrameworkMethod each : getAnnotatedMethods(annotationClass)) {
             try {
-                Object fieldValue = each.invokeExplosively(test);
-                if (valueClass.isInstance(fieldValue)) {
+                /*
+                 * A method annotated with @Rule may return a @TestRule or a @MethodRule,
+                 * we cannot call the method to check whether the return type matches our
+                 * expectation i.e. subclass of valueClass. If we do that then the method 
+                 * will be invoked twice and we do not want to do that. So we first check
+                 * whether return type matches our expectation and only then call the method
+                 * to fetch the MethodRule
+                 */
+                if (valueClass.isAssignableFrom(each.getReturnType())) {
+                    Object fieldValue = each.invokeExplosively(test);
                     results.add(valueClass.cast(fieldValue));
                 }
             } catch (Throwable e) {

Diff for: src/test/java/org/junit/tests/experimental/rules/MethodRulesTest.java

@@ -70,21 +70,20 @@ public void ruleIsIntroducedAndEvaluatedOnSubclass() {
 
     private static int runCount;
 
-    public static class MultipleRuleTest {
-        private static class Increment implements MethodRule {
-            public Statement apply(final Statement base,
-                    FrameworkMethod method, Object target) {
-                return new Statement() {
-                    @Override
-                    public void evaluate() throws Throwable {
-                        runCount++;
-                        base.evaluate();
-                    }
-
-                    ;
-                };
-            }
+    private static class Increment implements MethodRule {
+        public Statement apply(final Statement base,
+                FrameworkMethod method, Object target) {
+            return new Statement() {
+                @Override
+                public void evaluate() throws Throwable {
+                    runCount++;
+                    base.evaluate();
+                }
+            };
         }
+    }
+    
+    public static class MultipleRuleTest {
 
         @Rule
         public MethodRule incrementor1 = new Increment();
@@ -292,4 +291,122 @@ public void foo() {
     public void useCustomMethodRule() {
         assertThat(testResult(UsesCustomMethodRule.class), isSuccessful());
     }
+    
+    public static class HasMethodReturningMethodRule {
+        private MethodRule methodRule = new MethodRule() {
+            
+            @Override
+            public Statement apply(final Statement base, FrameworkMethod method, Object target) {
+                return new Statement() {
+                    
+                    @Override
+                    public void evaluate() throws Throwable {
+                        wasRun = true;
+                        base.evaluate();
+                    }
+                };
+            }
+        };
+        
+        @Rule
+        public MethodRule methodRule() {
+            return methodRule;
+        }
+        
+        @Test
+        public void doNothing() {
+            
+        }
+    }
+
+    /**
+     * If there are any public methods annotated with @Rule returning a {@link MethodRule}
+     * then it should also be run.
+     * 
+     * <p>This case has been added with 
+     * <a href="https://github.com/junit-team/junit/issues/589">Issue #589</a> - 
+     * Support @Rule for methods works only for TestRule but not for MethodRule
+     */
+    @Test
+    public void runsMethodRuleThatIsReturnedByMethod() {
+        wasRun = false;
+        JUnitCore.runClasses(HasMethodReturningMethodRule.class);
+        assertTrue(wasRun);
+    }
+    
+    public static class HasMultipleMethodsReturningMethodRule {
+        @Rule
+        public Increment methodRule1() {
+            return new Increment();
+        }
+        
+        @Rule
+        public Increment methodRule2() {
+            return new Increment();
+        }
+        
+        @Test
+        public void doNothing() {
+            
+        }
+    }
+
+    /**
+     * If there are multiple public methods annotated with @Rule returning a {@link MethodRule}
+     * then all the rules returned should be run.
+     * 
+     * <p>This case has been added with 
+     * <a href="https://github.com/junit-team/junit/issues/589">Issue #589</a> - 
+     * Support @Rule for methods works only for TestRule but not for MethodRule
+     */
+    @Test
+    public void runsAllMethodRulesThatAreReturnedByMethods() {
+        runCount = 0;
+        JUnitCore.runClasses(HasMultipleMethodsReturningMethodRule.class);
+        assertEquals(2, runCount);
+    }
+    
+    
+    public static class CallsMethodReturningRuleOnlyOnce {
+        int callCount = 0;
+        
+        private static class Dummy implements MethodRule {
+            
+            @Override
+            public Statement apply(final Statement base, FrameworkMethod method, Object target) {
+                return new Statement() {
+                    
+                    @Override
+                    public void evaluate() throws Throwable {
+                        base.evaluate();
+                    }
+                };
+            }
+        };
+       
+        
+        @Rule
+        public MethodRule methodRule() {
+            callCount++;
+            return new Dummy();
+        }
+        
+        @Test
+        public void doNothing() {
+            assertEquals(1, callCount);
+        }
+    }
+
+    /**
+     * If there are any public methods annotated with @Rule returning a {@link MethodRule}
+     * then method should be called only once.
+     * 
+     * <p>This case has been added with 
+     * <a href="https://github.com/junit-team/junit/issues/589">Issue #589</a> - 
+     * Support @Rule for methods works only for TestRule but not for MethodRule
+     */
+    @Test
+    public void callsMethodReturningRuleOnlyOnce() {
+        assertTrue(JUnitCore.runClasses(CallsMethodReturningRuleOnlyOnce.class).wasSuccessful());
+    }
 }