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

Prefer spies over mocks #23

Closed
andyw8 opened this issue Sep 5, 2014 · 14 comments
Closed

Prefer spies over mocks #23

andyw8 opened this issue Sep 5, 2014 · 14 comments
Assignees

Comments

@andyw8
Copy link
Collaborator

andyw8 commented Sep 5, 2014

I think a cop to verify that spies are used instead of stubs would be useful.

See http://robots.thoughtbot.com/spy-vs-spy

I'll try writing a PR for this if you agree with the principle.

@geniou
Copy link
Collaborator

geniou commented Sep 8, 2014

I like the idea. I'm not sure if this cop can be enabled by default, but I would add it the the selection. So feel free to add a pull request.

@nijikon nijikon self-assigned this Feb 18, 2016
@backus
Copy link
Collaborator

backus commented Aug 8, 2016

I think the best way for this to work as a cop is to use the EnforcedStyle convention rubocop has and just help enforce consistency (basically this gem shouldn't tell people whether to use spies or mocks). Essentially this should boil down to:

Prefer mocks

RSpec/MessageExpectation:
  EnforcedStyle: expect

Prefer spies

RSpec/MessageExpectation:
  EnforcedStyle: allow

@backus backus added the cop label Aug 8, 2016
backus added a commit that referenced this issue Aug 8, 2016
Enforces preferred styles of either

  expect(foo).to receive(:bar)

or

  allow(foo).to receive(:bar)

Fixes #23
@backus
Copy link
Collaborator

backus commented Aug 8, 2016

@geniou I've implemented this (#169) and I think it can be enabled by default. The cop supports EnforcedStyle and works with --auto-gen-config. Basically, if someone has inconsistent usage, --auto-gen-config can disable the cop in the user's .rubocop_todo.yml file.

@geniou
Copy link
Collaborator

geniou commented Aug 8, 2016

OK. Looking forward to see it in action. I'm not convinced that there is always only allow or expect - I think both have there use case, but I'll give it a try.

@jcoyne
Copy link

jcoyne commented Aug 30, 2016

I'm very disappointed to see this go in. When I want rspec to check something I use expect, but if I need an unchecked mock, I use allow. They are two different use cases.

jcoyne added a commit to samvera/hyku that referenced this issue Aug 30, 2016
Fixes #296

This is getting too heavy handed and imposing too much maintenence. Here
are a few examples:
rubocop/rubocop-rspec#149
rubocop/rubocop-rspec#94
rubocop/rubocop-rspec#23
@backus
Copy link
Collaborator

backus commented Aug 30, 2016

@jcoyne I'm happy to take your input on cops and even improve them if you think they are lacking. It is difficult to work with you though when your opening comments, without asking any questions about why we put time into these cops, are so negative.

@jcoyne
Copy link

jcoyne commented Aug 30, 2016

I'm sorry for being negative. It's a bitter pill to swallow when I update my bundle and all the builds start failing.

@backus
Copy link
Collaborator

backus commented Aug 30, 2016

@jcoyne thanks for apologizing. Likewise, you can imagine it being unpleasant to see these comments out of no where when I just try to add helpful cops in my free time.

With this all said, are you dead set on moving forward with your "Abandon rubocop-rspec" PR you've linked above or would you like me to continue to share context for these other cops? They are all implemented to be configurable and adjust to the user's preference. I'm happy to elaborate later if you're interested but I don't want to waste my time if you've written off this gem

@jcoyne
Copy link

jcoyne commented Aug 30, 2016

@backus I'm probably going to abandon rubocop-rspec on that one project, but I have at least a dozen projects that use it. (which is why changes are painful)

@backus
Copy link
Collaborator

backus commented Aug 30, 2016

@jcoyne that makes sense. You should look into rolling a gem that just has your shared rubocop configuration in it and also a rubocop + rubocop-rspec dependency. Then you could use rubocop's inherit_gem feature for inheriting shared config and then you would also not have as much of a problem of having surprise failures if you are only ever bumping the version of your rubocop dependency gem

@backus
Copy link
Collaborator

backus commented Aug 30, 2016

@jcoyne Glad to see you ended up sticking with the gem :) samvera/hyku#298

@jcoyne
Copy link

jcoyne commented Aug 30, 2016

@backus Yeah, tired and cranky last night ☺️ I'm better now.

bquorning added a commit that referenced this issue Oct 18, 2016
Instead of checking whether you create a stub or a mock, it now checks whether
you set message expectations using `receive` or `have_received`.

This relates to various issues:

 - #23
 - #169
 - #218
@bquorning bquorning mentioned this issue Oct 18, 2016
4 tasks
bquorning added a commit that referenced this issue Oct 18, 2016
Instead of checking whether you create a stub or a mock, it now checks whether
you set message expectations using `receive` or `have_received`.

This relates to various issues:

 - #23
 - #169
 - #218
bquorning added a commit that referenced this issue Oct 19, 2016
Instead of checking whether you create a stub or a mock, it now checks whether
you set message expectations using `receive` or `have_received`.

This relates to various issues:

 - #23
 - #169
 - #218
bquorning added a commit that referenced this issue Oct 19, 2016
Instead of checking whether you create a stub or a mock, it now checks whether
you set message expectations using `receive` or `have_received`.

This relates to various issues:

 - #23
 - #169
 - #218
@bquorning
Copy link
Collaborator

@jcoyne In #224 I’m rewriting the MessageExpectation cop to police receive vs have_received instead of policing allow vs expect. Your feedback would be much appreciated.

@jcoyne
Copy link

jcoyne commented Oct 24, 2016

@bquorning I like that (#224)!

bquorning added a commit that referenced this issue Oct 27, 2016
Check that message expectations are set using spies.

This relates to various issues:

 - #23
 - #169
 - #218
 - #229
bquorning added a commit that referenced this issue Oct 27, 2016
Users have expressed confusion over this cop; see

 - #23
 - #169
 - #218
 - #224
 - #229
bquorning added a commit that referenced this issue Oct 27, 2016
Users have expressed confusion over this cop; see

 - #23
 - #169
 - #218
 - #224
 - #229
bquorning added a commit that referenced this issue Oct 27, 2016
Check that message expectations are set using spies.

This relates to various issues:

 - #23
 - #169
 - #218
 - #229
bquorning added a commit that referenced this issue Oct 27, 2016
Check that message expectations are set using spies.

This relates to various issues:

 - #23
 - #169
 - #218
 - #229
bquorning added a commit that referenced this issue Nov 6, 2016
Check that message expectations are set using spies.

This relates to various issues:

 - #23
 - #169
 - #218
 - #229
andyw8 pushed a commit to andyw8/rubocop-rspec that referenced this issue Dec 17, 2016
Enforces preferred styles of either

  expect(foo).to receive(:bar)

or

  allow(foo).to receive(:bar)

Fixes rubocop#23
andyw8 pushed a commit to andyw8/rubocop-rspec that referenced this issue Dec 17, 2016
Users have expressed confusion over this cop; see

 - rubocop#23
 - rubocop#169
 - rubocop#218
 - rubocop#224
 - rubocop#229
andyw8 pushed a commit to andyw8/rubocop-rspec that referenced this issue Dec 17, 2016
Check that message expectations are set using spies.

This relates to various issues:

 - rubocop#23
 - rubocop#169
 - rubocop#218
 - rubocop#229
markburns pushed a commit to markburns/rubocop-rails-ddd that referenced this issue Nov 7, 2017
Enforces preferred styles of either

  expect(foo).to receive(:bar)

or

  allow(foo).to receive(:bar)

Fixes rubocop#23
markburns pushed a commit to markburns/rubocop-rails-ddd that referenced this issue Nov 7, 2017
Users have expressed confusion over this cop; see

 - rubocop#23
 - rubocop#169
 - rubocop#218
 - rubocop#224
 - rubocop#229
markburns pushed a commit to markburns/rubocop-rails-ddd that referenced this issue Nov 7, 2017
Check that message expectations are set using spies.

This relates to various issues:

 - rubocop#23
 - rubocop#169
 - rubocop#218
 - rubocop#229
pirj pushed a commit to rubocop/rubocop-capybara that referenced this issue Dec 29, 2022
pirj pushed a commit to rubocop/rubocop-capybara that referenced this issue Dec 29, 2022
Check that message expectations are set using spies.

This relates to various issues:

 - rubocop/rubocop-rspec#23
 - rubocop/rubocop-rspec#169
 - rubocop/rubocop-rspec#218
 - rubocop/rubocop-rspec#229
ydah pushed a commit to rubocop/rubocop-factory_bot that referenced this issue Apr 13, 2023
ydah pushed a commit to rubocop/rubocop-factory_bot that referenced this issue Apr 13, 2023
Check that message expectations are set using spies.

This relates to various issues:

 - rubocop/rubocop-rspec#23
 - rubocop/rubocop-rspec#169
 - rubocop/rubocop-rspec#218
 - rubocop/rubocop-rspec#229
ydah pushed a commit to rubocop/rubocop-rspec_rails that referenced this issue Mar 27, 2024
ydah pushed a commit to rubocop/rubocop-rspec_rails that referenced this issue Mar 27, 2024
Check that message expectations are set using spies.

This relates to various issues:

 - rubocop/rubocop-rspec#23
 - rubocop/rubocop-rspec#169
 - rubocop/rubocop-rspec#218
 - rubocop/rubocop-rspec#229
ydah pushed a commit to rubocop/rubocop-rspec_rails that referenced this issue Mar 27, 2024
ydah pushed a commit to rubocop/rubocop-rspec_rails that referenced this issue Mar 27, 2024
Check that message expectations are set using spies.

This relates to various issues:

 - rubocop/rubocop-rspec#23
 - rubocop/rubocop-rspec#169
 - rubocop/rubocop-rspec#218
 - rubocop/rubocop-rspec#229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants