From 46cecd6a870ccc0a0ec6ebda9bda82cb81782b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Kautler?= Date: Thu, 12 Mar 2020 14:43:30 +0100 Subject: [PATCH 1/2] Clean up RetryFeatureExtensionSpec --- .../RetryFeatureExtensionSpec.groovy | 320 +++++++----------- 1 file changed, 128 insertions(+), 192 deletions(-) diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy index 6e6825dba5..656d9a79da 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy @@ -15,21 +15,19 @@ class RetryFeatureExtensionSpec extends EmbeddedSpecification { static AtomicInteger featureCounter = new AtomicInteger() def setup() { + runner.addClassImport(Retry) + runner.addClassMemberImport(getClass()) runner.throwFailure = false featureCounter.set(0) } def "@Retry fails after retries are exhausted"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry - def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - expect: false - } + def result = runner.runSpecBody(""" +@Retry +def bar() { + featureCounter.incrementAndGet() + expect: false } """) @@ -47,16 +45,12 @@ class Foo extends Specification { def "@Retry works for normal exceptions"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry - def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - expect: - throw new IOException() - } + def result = runner.runSpecBody(""" +@Retry +def bar() { + featureCounter.incrementAndGet() + expect: + throw new IOException() } """) @@ -78,24 +72,20 @@ class Foo extends Specification { cleanupCounter.set(0) when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - def setup() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.setupCounter.incrementAndGet() - } + def result = runner.runSpecBody(""" +def setup() { + setupCounter.incrementAndGet() +} - @Retry(mode = Retry.Mode.${mode}) - def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - expect: - throw new IOException() - } +@Retry(mode = Retry.Mode.${mode}) +def bar() { + featureCounter.incrementAndGet() + expect: + throw new IOException() +} - def cleanup() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.cleanupCounter.incrementAndGet() - } +def cleanup() { + cleanupCounter.incrementAndGet() } """) @@ -120,15 +110,12 @@ class Foo extends Specification { def "@Retry count can be changed"() { when: - def result = runner.runWithImports("""import spock.lang.Retry - -class Foo extends Specification { - @Retry(count = 5) - def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - expect: - throw new Exception() - } + def result = runner.runSpecBody(""" +@Retry(count = 5) +def bar() { + featureCounter.incrementAndGet() + expect: + throw new Exception() } """) @@ -146,15 +133,11 @@ class Foo extends Specification { runner.throwFailure = true when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry(exceptions=[IndexOutOfBoundsException]) - def bar() { - expect: - throw new IllegalArgumentException() - } + def result = runner.runSpecBody(""" +@Retry(exceptions=[IndexOutOfBoundsException]) +def bar() { + expect: + throw new IllegalArgumentException() } """) @@ -167,18 +150,14 @@ class Foo extends Specification { runner.throwFailure = true when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry(exceptions=[IndexOutOfBoundsException]) - def bar() { - expect: - throw new IllegalArgumentException() - - where: - ignore << [1, 2] - } + def result = runner.runSpecBody(""" +@Retry(exceptions=[IndexOutOfBoundsException]) +def bar() { + expect: + throw new IllegalArgumentException() + + where: + ignore << [1, 2] } """) @@ -188,18 +167,14 @@ class Foo extends Specification { def "@Retry works for data driven features"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry - def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - expect: test + def result = runner.runSpecBody(""" +@Retry +def bar() { + featureCounter.incrementAndGet() + expect: test - where: - test << [false, false] - } + where: + test << [false, false] } """) @@ -212,18 +187,14 @@ class Foo extends Specification { def "@Retry for @Unroll'ed data driven feature"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry - def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - expect: test + def result = runner.runSpecBody(""" +@Retry +def bar() { + featureCounter.incrementAndGet() + expect: test - where: - test << [false, true, true] - } + where: + test << [false, true, true] } """) @@ -236,17 +207,13 @@ class Foo extends Specification { def "@Retry doesn't affect data driven feature where all iterations pass"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry - def bar() { - expect: test + def result = runner.runSpecBody(""" +@Retry +def bar() { + expect: test - where: - test << [true, true, true] - } + where: + test << [true, true, true] } """) @@ -258,17 +225,13 @@ class Foo extends Specification { def "@Retry doesn't affect @Unroll'ed data driven feature where all iterations pass"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry - def bar() { - expect: test + def result = runner.runSpecBody(""" +@Retry +def bar() { + expect: test - where: - test << [true, true, true] - } + where: + test << [true, true, true] } """) @@ -280,15 +243,11 @@ class Foo extends Specification { def "@Retry mode SETUP_FEATURE_CLEANUP ignores previous failures if a retry succeeds"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - static int counter - @Retry(mode = Retry.Mode.SETUP_FEATURE_CLEANUP) - def bar() { - expect: counter++ > 0 - } + def result = runner.runSpecBody(""" +static int counter +@Retry(mode = Retry.Mode.SETUP_FEATURE_CLEANUP) +def bar() { + expect: counter++ > 0 } """) @@ -301,18 +260,14 @@ class Foo extends Specification { def "@Retry can add delay between iteration executions"() { when: long start = System.currentTimeMillis() - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry(delay = 100) - def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - expect: test - - where: - test << [false, true, true] - } + def result = runner.runSpecBody(""" +@Retry(delay = 100) +def bar() { + featureCounter.incrementAndGet() + expect: test + + where: + test << [false, true, true] } """) @@ -329,19 +284,15 @@ class Foo extends Specification { def "@Retry evaluates condition"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry(condition = { failure.message.contains('1') }) - def "bar #message"() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - expect: - assert false : message - - where: - message << ['1', '2', '3'] - } + def result = runner.runSpecBody(""" +@Retry(condition = { failure.message.contains('1') }) +def "bar #message"() { + featureCounter.incrementAndGet() + expect: + assert false : message + + where: + message << ['1', '2', '3'] } """) @@ -354,25 +305,21 @@ class Foo extends Specification { def "@Retry does not evaluate condition if exception type is unexpected"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - @Retry(exceptions = IllegalArgumentException, condition = { failure.message.contains('1') }) - def "bar #exceptionClass #message"() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - expect: - throw exceptionClass.newInstance(message as String) - - where: - exceptionClass | message - IllegalArgumentException | 1 - IllegalArgumentException | 2 - IllegalStateException | 1 - IllegalStateException | 2 - RuntimeException | 1 - RuntimeException | 2 - } + def result = runner.runSpecBody(""" +@Retry(exceptions = IllegalArgumentException, condition = { failure.message.contains('1') }) +def "bar #exceptionClass #message"() { + featureCounter.incrementAndGet() + expect: + throw exceptionClass.newInstance(message as String) + + where: + exceptionClass | message + IllegalArgumentException | 1 + IllegalArgumentException | 2 + IllegalStateException | 1 + IllegalStateException | 2 + RuntimeException | 1 + RuntimeException | 2 } """) @@ -385,22 +332,18 @@ class Foo extends Specification { def "@Retry provides condition access to Specification instance"() { when: - def result = runner.runWithImports(""" -import spock.lang.Retry - -class Foo extends Specification { - int value - @Retry(condition = { instance.value == 2 }) - def "bar #input"() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() - value = input - - expect: - false - - where: - input << [1, 2, 3] - } + def result = runner.runSpecBody(""" +int value +@Retry(condition = { instance.value == 2 }) +def "bar #input"() { + featureCounter.incrementAndGet() + value = input + + expect: + false + + where: + input << [1, 2, 3] } """) @@ -414,23 +357,21 @@ class Foo extends Specification { def "@Retry can be declared on a spec class"() { when: def result = runner.runWithImports(""" -import spock.lang.Retry - @Retry class Foo extends Specification { def foo() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() + featureCounter.incrementAndGet() expect: false } def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() + featureCounter.incrementAndGet() expect: true } @Retry(count = 1) def baz() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() + featureCounter.incrementAndGet() expect: false } @@ -447,14 +388,12 @@ class Foo extends Specification { def "@Retry declared on a spec class is inherited"() { when: def result = runner.runWithImports(""" -import spock.lang.Retry - @Retry(count = 1) abstract class Foo extends Specification { } class Bar extends Foo { def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() + featureCounter.incrementAndGet() expect: false } @@ -471,11 +410,9 @@ class Bar extends Foo { def "@Retry declared on a subclass is applied to all features"() { when: def result = runner.runWithImports(""" -import spock.lang.Retry - abstract class Foo extends Specification { def foo() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() + featureCounter.incrementAndGet() expect: false } @@ -483,7 +420,7 @@ abstract class Foo extends Specification { @Retry(count = 1) class Bar extends Foo { def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() + featureCounter.incrementAndGet() expect: false } @@ -500,20 +437,19 @@ class Bar extends Foo { def "@Retry declared on a spec class can be overridden"() { when: def result = runner.runWithImports(""" -import spock.lang.Retry - @Retry(count = 1) abstract class Foo extends Specification { def foo() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() + featureCounter.incrementAndGet() expect: false } } + @Retry(count = 2) class Bar extends Foo { def bar() { - org.spockframework.smoke.extension.RetryFeatureExtensionSpec.featureCounter.incrementAndGet() + featureCounter.incrementAndGet() expect: false } From 1bd9edbee7e63970fd244f9e330adfe14745bf03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Kautler?= Date: Thu, 12 Mar 2020 15:43:55 +0100 Subject: [PATCH 2/2] Make @Retry repeatable (#1030) --- docs/extensions.adoc | 76 +++--------- docs/release_notes.adoc | 2 + .../extension/builtin/RetryExtension.java | 8 +- .../src/main/java/spock/lang/Retry.java | 11 ++ .../docs/extension/RetryDocSpec.groovy | 110 ++++++++++++++++++ .../RetryFeatureExtensionSpec.groovy | 29 +++++ 6 files changed, 171 insertions(+), 65 deletions(-) create mode 100644 spock-specs/src/test/groovy/org/spockframework/docs/extension/RetryDocSpec.groovy diff --git a/docs/extensions.adoc b/docs/extensions.adoc index 96249f4075..3da8336f46 100644 --- a/docs/extensions.adoc +++ b/docs/extensions.adoc @@ -263,59 +263,21 @@ It also provides special support for data driven features, offering to either re [source,groovy] ---- -class FlakyIntegrationSpec extends Specification { - @Retry - def retry3Times() { ... } - - @Retry(count = 5) - def retry5Times() { ... } - - @Retry(exceptions=[IOException]) - def onlyRetryIOException() { ... } - - @Retry(condition = { failure.message.contains('foo') }) - def onlyRetryIfConditionOnFailureHolds() { ... } - - @Retry(condition = { instance.field != null }) - def onlyRetryIfConditionOnInstanceHolds() { ... } - - @Retry - def retryFailingIterations() { - ... - where: - data << sql.select() - } - - @Retry(mode = Retry.Mode.FEATURE) - def retryWholeFeature() { - ... - where: - data << sql.select() - } - - @Retry(delay = 1000) - def retryAfter1000MsDelay() { ... } -} +include::{sourcedir}/extension/RetryDocSpec.groovy[tag=example-common] +include::{sourcedir}/extension/RetryDocSpec.groovy[tag=example-a] ---- Retries can also be applied to spec classes which has the same effect as applying it to each feature method that isn't -already annotated with {@code Retry}. +already annotated with `Retry`. [source,groovy] ---- -@Retry -class FlakyIntegrationSpec extends Specification { - def "will be retried with config from class"() { - ... - } - @Retry(count = 5) - def "will be retried using its own config"() { - ... - } -} +include::{sourcedir}/extension/RetryDocSpec.groovy[tag=example-b1] +include::{sourcedir}/extension/RetryDocSpec.groovy[tag=example-common] +include::{sourcedir}/extension/RetryDocSpec.groovy[tag=example-b2] ---- -A {@code @Retry} annotation that is declared on a spec class is applied to all features in all subclasses as well, +A `@Retry` annotation that is declared on a spec class is applied to all features in all subclasses as well, unless a subclass declares its own annotation. If so, the retries defined in the subclass are applied to all feature methods declared in the subclass as well as inherited ones. @@ -324,25 +286,15 @@ Running `BarIntegrationSpec` will execute `inherited` and `bar` with two retries [source,groovy] ---- -@Retry(count = 1) -abstract class AbstractIntegrationSpec extends Specification { - def inherited() { - ... - } -} +include::{sourcedir}/extension/RetryDocSpec.groovy[tag=example-c] +---- -class FooIntegrationSpec extends AbstractIntegrationSpec { - def foo() { - ... - } -} +If multiple `@Retry` annotations are present, they can be used to have different retry settings +for different situations: -@Retry(count = 2) -class BarIntegrationSpec extends AbstractIntegrationSpec { - def bar() { - ... - } -} +[source,groovy,indent=0] +---- +include::{sourcedir}/extension/RetryDocSpec.groovy[tag=example-d] ---- Check https://github.com/spockframework/spock/blob/master/spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy[RetryFeatureExtensionSpec] for more examples. diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc index c68fc8a46b..3b63a85aa2 100644 --- a/docs/release_notes.adoc +++ b/docs/release_notes.adoc @@ -16,6 +16,8 @@ include::include.adoc[] - `@Issue` is now repeatable +- `@Retry` is now repeatable + == 2.0-M3 (2020-06-11) diff --git a/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/RetryExtension.java b/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/RetryExtension.java index 4733e7a9c8..24f8f7ee8f 100644 --- a/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/RetryExtension.java +++ b/spock-core/src/main/java/org/spockframework/runtime/extension/builtin/RetryExtension.java @@ -20,17 +20,19 @@ import org.spockframework.runtime.model.*; import spock.lang.Retry; +import java.util.List; + /** * @author Leonard Brünings * @since 1.2 */ public class RetryExtension implements IAnnotationDrivenExtension { @Override - public void visitSpecAnnotation(Retry annotation, SpecInfo spec) { + public void visitSpecAnnotations(List annotations, SpecInfo spec) { if (noSubSpecWithRetryAnnotation(spec.getSubSpec())) { for (FeatureInfo feature : spec.getBottomSpec().getAllFeatures()) { if (noRetryAnnotation(feature.getFeatureMethod())) { - visitFeatureAnnotation(annotation, feature); + visitFeatureAnnotations(annotations, feature); } } } @@ -44,7 +46,7 @@ private boolean noSubSpecWithRetryAnnotation(SpecInfo spec) { } private boolean noRetryAnnotation(NodeInfo node) { - return !node.getReflection().isAnnotationPresent(Retry.class); + return !node.isAnnotationPresent(Retry.class); } @Override diff --git a/spock-core/src/main/java/spock/lang/Retry.java b/spock-core/src/main/java/spock/lang/Retry.java index 0a9f9ca67c..85391b4dc8 100644 --- a/spock-core/src/main/java/spock/lang/Retry.java +++ b/spock-core/src/main/java/spock/lang/Retry.java @@ -44,6 +44,7 @@ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE, ElementType.METHOD}) @ExtensionAnnotation(RetryExtension.class) +@Repeatable(Retry.Container.class) public @interface Retry { /** * Configures which types of Exceptions should be retried. @@ -103,4 +104,14 @@ enum Mode { */ SETUP_FEATURE_CLEANUP } + + /** + * @since 2.0 + */ + @Beta + @Retention(RetentionPolicy.RUNTIME) + @Target({ElementType.TYPE, ElementType.METHOD}) + @interface Container { + Retry[] value(); + } } diff --git a/spock-specs/src/test/groovy/org/spockframework/docs/extension/RetryDocSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/docs/extension/RetryDocSpec.groovy new file mode 100644 index 0000000000..3d0a0a4902 --- /dev/null +++ b/spock-specs/src/test/groovy/org/spockframework/docs/extension/RetryDocSpec.groovy @@ -0,0 +1,110 @@ +package org.spockframework.docs.extension + +import groovy.sql.Sql +import spock.lang.Retry +import spock.lang.Shared +import spock.lang.Specification + +abstract +// tag::example-common[] +class FlakyIntegrationSpec extends Specification { +// end::example-common[] + @Shared + def sql = Sql.newInstance("jdbc:h2:mem:", "org.h2.Driver") + +// tag::example-d[] + @Retry(exceptions = IllegalArgumentException, count = 2) + @Retry(exceptions = IllegalAccessException, count = 4) + def retryDependingOnException() { +// end::example-d[] + expect: true + } +} + +class FlakyIntegrationSpecA extends FlakyIntegrationSpec { +// tag::example-a[] + @Retry + def retry3Times() { + expect: true + } + + @Retry(count = 5) + def retry5Times() { + expect: true + } + + @Retry(exceptions = [IOException]) + def onlyRetryIOException() { + expect: true + } + + @Retry(condition = { failure.message.contains('foo') }) + def onlyRetryIfConditionOnFailureHolds() { + expect: true + } + + @Retry(condition = { instance.field != null }) + def onlyRetryIfConditionOnInstanceHolds() { + expect: true + } + + @Retry + def retryFailingIterations() { + expect: true + + where: + data << sql.execute('') + } + + @Retry(mode = Retry.Mode.SETUP_FEATURE_CLEANUP) + def retryWholeFeature() { + expect: true + + where: + data << sql.execute('') + } + + @Retry(delay = 1000) + def retryAfter1000MsDelay() { + expect: true + } +} +// end::example-a[] + +// tag::example-b1[] +@Retry +// end::example-b1[] +class FlakyIntegrationSpecB extends FlakyIntegrationSpec { +// tag::example-b2[] + def "will be retried with config from class"() { + expect: true + } + + @Retry(count = 5) + def "will be retried using its own config"() { + expect: true + } +} +// end::example-b2[] + +// tag::example-c[] +@Retry(count = 1) +abstract class AbstractIntegrationSpec extends Specification { + def inherited() { + expect: true + } +} + +class FooIntegrationSpec extends AbstractIntegrationSpec { + def foo() { + expect: true + } +} + +@Retry(count = 2) +class BarIntegrationSpec extends AbstractIntegrationSpec { + def bar() { + expect: true + } +} +// end::example-c[] diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy index 656d9a79da..ba73439879 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy @@ -66,6 +66,35 @@ def bar() { featureCounter.get() == 4 } + def "@Retry works properly if applied multiple times"() { + when: + def result = runner.runSpecBody(""" +@Retry(exceptions = IllegalArgumentException, count = 2) +@Retry(exceptions = IllegalAccessException, count = 4) +def bar() { + featureCounter.incrementAndGet() + expect: + throw new ${exception.simpleName}() +} + """) + + then: + result.testsStartedCount == 1 + result.testsSucceededCount == 0 + result.testsFailedCount == 1 + with(result.failures.exception[0], MultipleFailuresError) { + failures.size() == expectedCount + failures.every { exception.isInstance(it) } + } + result.testsSkippedCount == 0 + featureCounter.get() == expectedCount + + where: + exception || expectedCount + IllegalArgumentException || 3 + IllegalAccessException || 5 + } + def "@Retry mode #mode executes setup and cleanup #expectedCount times"(String mode, int expectedCount) { given: setupCounter.set(0)