Merge pull request #5 from anotherchrisberry/intercept-setup-cleanup

catch exceptions in setup/cleanup methods

Author: anotherchrisberry

Diff for: .gitignore

@@ -2,8 +2,13 @@
 .gradle
 out
 *.iml
+*.ipr
+*.iws
 build
+gradle
+gradlew
+gradlew.bat
 .classpath
 .project
-.settings/
-bin/
+.settings
+bin

Diff for: build.gradle

@@ -1,6 +1,7 @@
 apply plugin: "groovy"
 apply plugin: "maven"
 apply plugin: "maven-publish"
+apply plugin: "idea"
 
 repositories() {
   mavenCentral()

Diff for: src/main/groovy/com/anotherchrisberry/spock/extensions/retry/RetryInterceptor.groovy

@@ -30,13 +30,23 @@ class RetryInterceptor implements IMethodInterceptor {
                     throw t
                 }
                 invocation.spec.cleanupMethods.each {
-                    if (it.reflection) {
-                        ReflectionUtil.invokeMethod(invocation.target, it.reflection)
+                    try {
+                        if (it.reflection) {
+                            ReflectionUtil.invokeMethod(invocation.target, it.reflection)
+                        }
+                    } catch (Throwable t2) {
+                        LOG.warn("Retry caught failure ${attempts + 1} / ${retryMax + 1} while cleaning up", t2)
                     }
                 }
                 invocation.spec.setupMethods.each {
-                    if (it.reflection) {
-                        ReflectionUtil.invokeMethod(invocation.target, it.reflection)
+                    try {
+                        if (it.reflection) {
+                            ReflectionUtil.invokeMethod(invocation.target, it.reflection)
+                        }
+                    } catch (Throwable t2) {
+                        // increment counter, since this is the start of the re-run
+                        attempts++
+                        LOG.info("Retry caught failure ${attempts + 1} / ${retryMax + 1} while setting up", t2)
                     }
                 }
             }

Diff for: src/main/groovy/com/anotherchrisberry/spock/extensions/retry/RetrySpecExtension.groovy

@@ -1,7 +1,9 @@
 package com.anotherchrisberry.spock.extensions.retry
 
+import org.spockframework.compiler.model.FeatureMethod
 import org.spockframework.runtime.extension.AbstractAnnotationDrivenExtension
 import org.spockframework.runtime.model.FeatureInfo
+import org.spockframework.runtime.model.MethodInfo
 import org.spockframework.runtime.model.SpecInfo
 
 class RetrySpecExtension extends AbstractAnnotationDrivenExtension<RetryOnFailure> {
@@ -31,7 +33,7 @@ class RetrySpecExtension extends AbstractAnnotationDrivenExtension<RetryOnFailur
             List<FeatureInfo> featuresToRetry = [selfAndSubSpecs.features].flatten().unique()
             for (FeatureInfo feature : featuresToRetry) {
                 clearInterceptors(feature)
-                feature.getFeatureMethod().addInterceptor(new RetryInterceptor(getNumberOfRetries(retries)))
+                addInterceptors(feature, retries)
             }
         }
     }
@@ -41,7 +43,25 @@ class RetrySpecExtension extends AbstractAnnotationDrivenExtension<RetryOnFailur
         return Integer.parseInt(System.getProperty("spock-retry.times", defaultRetries))
     }
 
+    private List<MethodInfo> getInterceptableMethods(FeatureInfo feature) {
+        SpecInfo spec = feature.getSpec()
+        [ spec.setupMethods,
+          spec.setupSpecMethods,
+          spec.cleanupMethods,
+          spec.cleanupSpecMethods,
+          feature.featureMethod
+        ].flatten().unique() as List<MethodInfo>
+    }
+
     private void clearInterceptors(FeatureInfo featureInfo) {
-        featureInfo.featureMethod.interceptors.removeAll { it.class == RetryInterceptor }
+        List<MethodInfo> interceptableMethods = getInterceptableMethods(featureInfo)
+        interceptableMethods.each { it.interceptors.removeAll { it.class == RetryInterceptor } }
+    }
+
+    private void addInterceptors(FeatureInfo featureInfo, RetryOnFailure retries) {
+        def interceptor = new RetryInterceptor(getNumberOfRetries(retries))
+        getInterceptableMethods(featureInfo).each {
+            it.addInterceptor(interceptor)
+        }
     }
 }

Diff for: src/test/groovy/com/anotherchrisberry/spock/extensions/retry/RetryOnFailureCleanupSpec.groovy

@@ -0,0 +1,29 @@
+package com.anotherchrisberry.spock.extensions.retry
+
+import spock.lang.Specification
+
+@RetryOnFailure(times=2)
+class RetryOnFailureCleanupSpec extends Specification {
+
+  static Integer cleanupCalls = 0
+
+  void cleanup() {
+    if (cleanupCalls < 2) {
+      cleanupCalls++
+      throw new RuntimeException("have not tried enough times ($cleanupCalls)")
+    }
+  }
+
+  void 'class level test'() {
+    given:
+    Mockable mockable = Mock()
+
+    when:
+    mockable.doMockyThing()
+
+    then:
+    cleanupCalls == 2
+    1 * mockable.doMockyThing()
+    0 * _
+  }
+}

Diff for: src/test/groovy/com/anotherchrisberry/spock/extensions/retry/RetryOnFailureCleanupSubclassSpec.groovy

@@ -0,0 +1,21 @@
+package com.anotherchrisberry.spock.extensions.retry
+
+class RetryOnFailureCleanupSubclassSpec extends RetryOnFailureCleanupSpec {
+
+  void cleanupSpec() {
+    cleanupCalls = 0
+  }
+
+  void 'should call parent setup method'() {
+    given:
+    Mockable mockable = Mock()
+
+    when:
+    mockable.doMockyThing()
+
+    then:
+    cleanupCalls == 2
+    1 * mockable.doMockyThing()
+    0 * _
+  }
+}

Diff for: src/test/groovy/com/anotherchrisberry/spock/extensions/retry/RetryOnFailureSetupSpec.groovy

@@ -0,0 +1,29 @@
+package com.anotherchrisberry.spock.extensions.retry
+
+import spock.lang.Specification
+
+@RetryOnFailure(times=2)
+class RetryOnFailureSetupSpec extends Specification {
+
+    static Integer setupCalls = 0
+
+    void setup() {
+        if (setupCalls < 2) {
+            setupCalls++
+            throw new RuntimeException("have not tried enough times ($setupCalls)")
+        }
+    }
+
+    void 'class level test'() {
+        given:
+        Mockable mockable = Mock()
+
+        when:
+        mockable.doMockyThing()
+
+        then:
+        setupCalls == 2
+        1 * mockable.doMockyThing()
+        0 * _
+    }
+}

Diff for: src/test/groovy/com/anotherchrisberry/spock/extensions/retry/RetryOnFailureSetupSubclassSpec.groovy

@@ -0,0 +1,36 @@
+package com.anotherchrisberry.spock.extensions.retry
+
+@RetryOnFailure(times=5)
+class RetryOnFailureSetupSubclassSpec extends RetryOnFailureSetupSpec {
+
+  static Integer specCalls = 0
+
+  void setupSpec() {
+    setupCalls = 0
+  }
+
+  void 'should call parent setup method'() {
+    given:
+    Mockable mockable = Mock()
+
+    when:
+    mockable.doMockyThing()
+
+    then:
+    setupCalls == 2
+    1 * mockable.doMockyThing()
+    0 * _
+  }
+
+  void 'should still try to run own method when setup fails in superclass'() {
+    when:
+    if (specCalls < 3) {
+      specCalls++
+      throw new RuntimeException("have not tried enough times ($specCalls)")
+    }
+
+    then:
+    setupCalls == 2
+    specCalls == 3
+  }
+}