-
Notifications
You must be signed in to change notification settings - Fork 470
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 @PendingFeature repeatable (#1030) #1190
base: master
Are you sure you want to change the base?
Make @PendingFeature repeatable (#1030) #1190
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1190 +/- ##
=========================================
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.
|
9116297
to
1290fa7
Compare
|
||
// tag::example-b[] | ||
@PendingFeature( | ||
exceptions = UnsupportedOperationException, |
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 include an example that has a list of exceptions
def "@PendingFeature marks failing feature as skipped even if applied twice"() { | ||
when: | ||
def result = runner.runSpecBody """ | ||
@PendingFeature |
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.
While not "wrong", it is redundant, we should check for redundant annotations, as the user probably intended something different.
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.
The question is, how much logic to stuff in for those redundancy checks and how much to trust the user.
You can also prevent multiple @Issue
annotations with the same value or.
Or you could even prevent @Issue(['1', '2'], ['2', '3'])
as the '2'
is redundant.
Same for two @PendingFeature
annotations with the same expected exception in their lists.
when: | ||
def result = runner.runSpecBody """ | ||
@PendingFeature(reason='42') | ||
@PendingFeature(reason='4711') |
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.
This looks even more like an error
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.
While there is probably no sensible use-case for this in the wild, the test is just intended to ensure a stable reason being shown in the error message. As for the redundancy-check or whatever, let's continue above.
1290fa7
to
8d46d79
Compare
This change is