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 sure role group delegates .WHY to its default candidate #783

Merged
merged 2 commits into from
Jan 6, 2022

Conversation

vrurg
Copy link
Contributor

@vrurg vrurg commented Jan 6, 2022

This required to remove tests which were testing for Role.WHY returning Nil. But as was suggested in this comment:

rakudo/rakudo#3549 (comment)

the tests were needed to ensure that a documentation comment is attached to a particular parametric role, not to its group. But since rakudo/rakudo#3549 makes .WHY delegate to the default candidate, these tests doesn't make sense anymore. Instead why-both.t now makes sure what we get from a role group is the same object we get from the default candidate.

Alongside with the essential changes, also unified all tests with regard to use of idential test assertion subs. They all now are subtest based.

This required to remove tests which were testing for `Role.WHY`
returning `Nil`. But as was suggested in this comment:

rakudo/rakudo#3549 (comment)

the tests were needed to ensure that a documentation comment is attached
to a particular parametric role, not to its group. But since
rakudo/rakudo#3549 makes `.WHY` delegate to the default candidate, these
tests doesn't make sense anymore. Instead why-both.t now makes sure what
we get from a role group is the same object we get from the default
candidate.

Alongside with the essential changes, also unified all tests with regard
to use of idential test assertion subs. They all now are `subtest`
based.
It is passing since rakudo/rakudo#3549
@vrurg vrurg merged commit 250a246 into master Jan 6, 2022
@Altai-man Altai-man deleted the WHY-on-role-group branch January 7, 2022 11:31
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.

None yet

1 participant