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 @PendingFeature repeatable (#1030) #1190

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Vampire
Copy link
Member

@Vampire Vampire commented Jul 9, 2020

This change is Reviewable

@codecov
Copy link

codecov bot commented Jul 9, 2020

Codecov Report

Merging #1190 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3aefd7...a464c28. Read the comment docs.

@Vampire Vampire force-pushed the repeatable-pendingfeature-annotation branch 3 times, most recently from 9116297 to 1290fa7 Compare July 13, 2020 19:41

// tag::example-b[]
@PendingFeature(
exceptions = UnsupportedOperationException,
Copy link
Member

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
Copy link
Member

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.

Copy link
Member Author

@Vampire Vampire Jul 14, 2020

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')
Copy link
Member

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

Copy link
Member Author

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.

@Vampire Vampire force-pushed the repeatable-pendingfeature-annotation branch from 1290fa7 to 8d46d79 Compare July 14, 2020 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants