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

[Turn Off] ExpectInHook #5

Open
juriglx opened this issue Feb 13, 2018 · 2 comments
Open

[Turn Off] ExpectInHook #5

juriglx opened this issue Feb 13, 2018 · 2 comments

Comments

@juriglx
Copy link

juriglx commented Feb 13, 2018

I don't like ExpectInHook

I think it is useful to setup something like:

before { expect(obj).to receive(:foo) }

The laziest solution, because we also have only one expect per spec, is to change it to:

before { allow(obj).to receive(:foo) }

But this weakens the test requirements, which should not be an effect of rubocop.

@fnoeh
Copy link

fnoeh commented Feb 14, 2018

Agree.

@fanaugen
Copy link
Contributor

I think that cop is useful: expectations belong in it or specify blocks, not in before or after hooks. imho the pattern when you stub a method but want to assert that the method is called, should be like this (even at the cost of duplication):

# ↓ before block, or inside example
allow(obj).to receive(:foo).and_return(:bar)

# ↓ inside example
expect(obj).to have_received(:foo)

because we also have only one expect per spec

I think ↑ that is the cop that is more harmful than useful. It appears to be less strict in loveos-backend, where 2 expects are allowed per example. My vote is to turn off RSpec/MultipleExpectations altogether and rely on people’s best judgment not to put 100 expects into one it (but even that can in rare cases be acceptable, e.g. in a loop?)

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

No branches or pull requests

3 participants