-
Notifications
You must be signed in to change notification settings - Fork 476
Fix the @Retry documentation #2174
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,7 +28,7 @@ | |||||||
|
|
||||||||
|
|
||||||||
| /** | ||||||||
| * Retries the given feature if an exception occurs during execution. | ||||||||
| * Retries the given feature if an exception occurs during execution of the feature method. | ||||||||
| * | ||||||||
| * <p>Retries can be applied to feature methods and spec classes. Applying it to | ||||||||
| * a spec class has the same effect as applying it to each feature method that | ||||||||
|
|
@@ -50,7 +50,7 @@ | |||||||
| /** | ||||||||
| * Configures which types of Exceptions should be retried. | ||||||||
| * | ||||||||
| * Subclasses are included if their parent class is listed. | ||||||||
| * <p>Subclasses are included if their parent class is listed. | ||||||||
| * | ||||||||
| * @return array of Exception classes to retry. | ||||||||
| */ | ||||||||
|
|
@@ -77,12 +77,12 @@ Class<? extends Throwable>[] skipRetryExceptions() default { | |||||||
| * Condition that is evaluated to decide whether the feature should be | ||||||||
| * retried. | ||||||||
| * | ||||||||
| * The configured closure is called with a delegate of type | ||||||||
| * <p>The configured closure is called with a delegate of type | ||||||||
| * {@link org.spockframework.runtime.extension.builtin.RetryConditionContext} | ||||||||
| * which provides access to the current exception and {@code Specification} | ||||||||
| * instance. | ||||||||
| * | ||||||||
| * The feature is retried if the exception class passes the type check and the | ||||||||
| * <p>The feature is retried if the exception class passes the type check and the | ||||||||
| * specified condition holds true. If no condition is specified, only the type | ||||||||
| * check is performed. | ||||||||
| * | ||||||||
|
|
@@ -113,12 +113,14 @@ Class<? extends Throwable>[] skipRetryExceptions() default { | |||||||
|
|
||||||||
| enum Mode { | ||||||||
| /** | ||||||||
| * Retry the iterations individually. | ||||||||
| * Retry only the feature method execution, setup and cleanup are not running on retries. | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we also the the same sentence as in the docu
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it makes sense here. The sentence in the docs is about the In the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Besides that it is anyway questionable whether this is actually the intended behavior, thus my question above to @leonard84 |
||||||||
| */ | ||||||||
| ITERATION, | ||||||||
|
|
||||||||
| /** | ||||||||
| * Retry the feature together with the setup and cleanup methods. | ||||||||
| * Retry the iteration together with setup and cleanup. | ||||||||
| * Even in this mode, the retry is only triggered if the feature method is failing | ||||||||
| * in the expected way. If the setup or cleanup is failing, the test fails immediately. | ||||||||
| */ | ||||||||
| SETUP_FEATURE_CLEANUP | ||||||||
| } | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| 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") | ||
| } | ||
|
|
||
| class FlakyIntegrationSpecA extends FlakyIntegrationSpec { | ||
| // tag::example-a[] | ||
| @Retry | ||
| def "retry 3 times"() { | ||
| expect: true | ||
| } | ||
|
|
||
| @Retry(count = 5) | ||
| def "retry 5 times"() { | ||
| expect: true | ||
| } | ||
|
|
||
| @Retry(exceptions = [IOException]) | ||
| def "only retry on IOException"() { | ||
| expect: true | ||
| } | ||
|
|
||
| @Retry(condition = { failure.message.contains('foo') }) | ||
| def "only retry if condition on failure holds"() { | ||
| expect: true | ||
| } | ||
|
|
||
| @Retry(condition = { instance.field != null }) | ||
| def "only retry if condition on instance holds"() { | ||
| expect: true | ||
| } | ||
|
|
||
| @Retry | ||
| def "retry failing feature methods"() { | ||
| expect: true | ||
|
|
||
| where: | ||
| data << sql.execute('') | ||
| } | ||
|
|
||
| @Retry(mode = Retry.Mode.SETUP_FEATURE_CLEANUP) | ||
| def "retry with setup and cleanup"() { | ||
| expect: true | ||
|
|
||
| where: | ||
| data << sql.execute('') | ||
| } | ||
|
|
||
| @Retry(delay = 1000) | ||
| def "retry after 1000 ms delay"() { | ||
| 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[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leonard84 is this actually the intended and expected behavior?
That only expected failures in the feature method trigger a retry I mean.
I do some UI tests using Geb with Marathon driver and it happens sometimes on GHA that the application cannot be started and thus the driver throws an exception.
This is done in an iteration interceptor before calling
proceed, the iteration interceptor is within the retry iteration interceptor due to using global retry extension. But as the error happens there and not during feature method invocation, it does not cause a retry.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the current behavior. I'm not opposed to letting it cover
setupas well.Though if I'm not mistaken, you'll have problems covering
cleanupwithout the method executions appearing in the results.