Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make @Retry repeatable (#1030) #1197

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 14 additions & 62 deletions docs/extensions.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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]
----
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we want code backed docs where ever possible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All pimped

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.
Expand Down
2 changes: 2 additions & 0 deletions docs/release_notes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ include::include.adoc[]

- `@Issue` is now repeatable

- `@Retry` is now repeatable


== 2.0-M3 (2020-06-11)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Retry> {
@Override
public void visitSpecAnnotation(Retry annotation, SpecInfo spec) {
public void visitSpecAnnotations(List<Retry> annotations, SpecInfo spec) {
if (noSubSpecWithRetryAnnotation(spec.getSubSpec())) {
for (FeatureInfo feature : spec.getBottomSpec().getAllFeatures()) {
if (noRetryAnnotation(feature.getFeatureMethod())) {
visitFeatureAnnotation(annotation, feature);
visitFeatureAnnotations(annotations, feature);
}
}
}
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about Retry.Container?

}

@Override
Expand Down
11 changes: 11 additions & 0 deletions spock-core/src/main/java/spock/lang/Retry.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a test, that actually alternately throws both these exceptions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is unfortunately a good idea, though in the test spec, not the doc spec.
Unfortunately, because I think the changes were too naive for the retry extension logic to properly work in that case in its current state. I have to rework that probably when I'm a bit less sleepy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you might have a look, as you invented the whole extension and probably know the code better than me.

@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[]
Loading