-
Notifications
You must be signed in to change notification settings - Fork 471
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
base: master
Are you sure you want to change the base?
Make @Retry repeatable (#1030) #1197
Conversation
for different situations: | ||
|
||
[source,groovy] | ||
---- |
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.
we want code backed docs where ever possible
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.
All pimped
Codecov Report
@@ Coverage Diff @@
## master #1197 +/- ##
=========================================
Coverage 75.96% 75.96%
Complexity 3647 3647
=========================================
Files 393 393
Lines 11113 11113
Branches 1369 1369
=========================================
Hits 8442 8442
Misses 2192 2192
Partials 479 479
Continue to review full report at Codecov.
|
d2e2b5e
to
3b4d2e3
Compare
3b4d2e3
to
1bd9edb
Compare
def sql = Sql.newInstance("jdbc:h2:mem:", "org.h2.Driver") | ||
|
||
// tag::example-d[] | ||
@Retry(exceptions = IllegalArgumentException, count = 2) |
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.
Should we add a test, that actually alternately throws both these exceptions?
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.
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.
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.
Or you might have a look, as you invented the whole extension and probably know the code better than me.
@@ -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); |
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.
What about Retry.Container
?
Make @Retry repeatable (#1030)
This change is