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

negated matcher for be_able_to only passes when no ability is eligible #646

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

mraidel
Copy link

@mraidel mraidel commented Aug 24, 2020

No description provided.

@mraidel
Copy link
Author

mraidel commented Aug 24, 2020

Issue with description: #645

@mraidel mraidel force-pushed the negated_matcher_for_be_able_to branch from ceaf15f to b439cc0 Compare August 24, 2020 12:18
@23tux
Copy link

23tux commented Oct 29, 2020

What's the state of this PR? Can it be merged?

CHANGELOG.md Outdated Show resolved Hide resolved
@mraidel mraidel force-pushed the negated_matcher_for_be_able_to branch 2 times, most recently from f8494f3 to 6f5b529 Compare November 2, 2020 14:47
@mraidel
Copy link
Author

mraidel commented Nov 2, 2020

@ghiculescu thanks for the comment, I have updated the PR with a squashed commit including the correct commit message in the changelog, also resolved the merge conflict.

@mraidel mraidel force-pushed the negated_matcher_for_be_able_to branch from 6f5b529 to c1c6cca Compare November 4, 2020 07:27
@coorasse
Copy link
Member

coorasse commented Apr 28, 2021

related to #505 and #602

@coorasse
Copy link
Member

I think we discussed this issue too often. But something changed recently: I found out this behaviour in RSpec, you can define:

    expect([1, 3, 5, 7]).to all(be_odd)
    expect([1, 3, 5, 7]).not_to all(be_even)

this is very similar to what we are discussing here. Guess what does RSpec do? It raises

NotImplementedError: `expect().not_to all( matcher )` is not supported.

I think we should do the same

@coorasse
Copy link
Member

Sorry @mraidel , I was in a rush (and maybe a bit rude 😅 ) in my comment above.

I really appreciate the effort you did put in this PR 🙏 . I'd love to get your feedback regarding my proposal.

@mraidel
Copy link
Author

mraidel commented May 16, 2021

Hi @coorasse, I understand the RSpec behavior for "all" because it mirrors the use of the language, "not all" just doesn't mean "none". I don't see the same issue in ".not_to be_able_to" because in this case for me the new behavior also mirrors the language. If I say "I don't want to be able to do these things" I would expect that I can do none of these, right? An alternative solution would be to disallow the negated matcher and have a new matcher "be_unable_to". Maybe there is still some possible ambiguity because one could interpret it either as "not be able to do a and b" or "not to be able to do a or b". In this case the only solution I would see is to disallow using the negated matcher with more than one argument. Just disallowing the negated matcher would miss any way to test for someone unable to access something which I think is a quite important thing to test.

@coorasse
Copy link
Member

I would disallow it only with multiple actions. You would still be able to run it with a single action.

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.

4 participants