-
Notifications
You must be signed in to change notification settings - Fork 475
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
base: master
Are you sure you want to change the base?
Conversation
0fbb1a3 to
ad388ae
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2174 +/- ##
============================================
- Coverage 82.07% 82.06% -0.01%
+ Complexity 4756 4755 -1
============================================
Files 465 465
Lines 14871 14871
Branches 1878 1878
============================================
- Hits 12205 12204 -1
Misses 1978 1978
- Partials 688 689 +1
🚀 New features to boost your workflow:
|
592f7a9 to
a8dd99c
Compare
| In addition, an optional `condition` closure can be used to determine if a feature should be retried. | ||
| It also provides special support for data driven features, offering to either retry all iterations or just the failing ones. | ||
| In its standard mode it only retries the feature method execution, but this can be changed using `mode` to | ||
| also run setup and cleanup on retries. Even in this mode, the retry is only triggered if the feature method is failing |
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 setup as well.
Though if I'm not mistaken, you'll have problems covering cleanup without the method executions appearing in the results.
a8dd99c to
e11ba73
Compare
e11ba73 to
801f260
Compare
48c6bdc to
3022569
Compare
3a690d8 to
6efbe05
Compare
| enum Mode { | ||
| /** | ||
| * Retry the iterations individually. | ||
| * Retry only the feature method execution, setup and cleanup are not running on retries. |
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.
Maybe we also the the same sentence as in the docu
| * Retry only the feature method execution, setup and cleanup are not running on retries. | |
| * Retry only the feature method execution, setup and cleanup are not running on retries. | |
| * If the setup or cleanup is failing, the test fails immediately. |
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.
I don't think it makes sense here.
The sentence in the docs is about the SETUP_FEATURE_CLEANUP mode below and there I already added the sentence like in the docs.
In the ITERATION mode setup and cleanup are not part of the retry, so why should we additionally mention that the test fails if they are failing, they are not affected in any way by the retry anyway in that mode.
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.
Besides that it is anyway questionable whether this is actually the intended behavior, thus my question above to @leonard84
spock-specs/src/test/groovy/org/spockframework/smoke/extension/RetryFeatureExtensionSpec.groovy
Outdated
Show resolved
Hide resolved
1b03137 to
1d6da54
Compare
| In addition, an optional `condition` closure can be used to determine if a feature should be retried. | ||
| It also provides special support for data driven features, offering to either retry all iterations or just the failing ones. | ||
| In its standard mode it only retries the feature method execution, but this can be changed using `mode` to | ||
| also run setup and cleanup on retries. Even in this mode, the retry is only triggered if the feature method is failing |
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 setup as well.
Though if I'm not mistaken, you'll have problems covering cleanup without the method executions appearing in the results.
1d6da54 to
8423afe
Compare
8423afe to
989e118
Compare

No description provided.