rubocop / rubocop-rspec

Code style checking for RSpec files.
https://docs.rubocop.org/rubocop-rspec
MIT License
810 stars 276 forks source link

Cop idea: detect let that have to be a subject #401

Open Darhazer opened 7 years ago

Darhazer commented 7 years ago

Example code:

RSpec.describe User do
  context 'admin'
    let(:admin) { create :user, role: admin }
    it '...' do
      expect(admin.something).to eq(result)
    end
end

Generally when something is used in an expect, is should be the subject of a test, not a let Exception may be when the thing is a mock, e.g. in expect(admin).to receive(...) calls

mvz commented 6 years ago

@Darhazer what would be the suggested 'good' version of your example?

Darhazer commented 6 years ago
RSpec.describe User do
  context 'admin'
    subject(:admin) { create :user, role: admin }
    it '...' do
      expect(subject.something).to eq(result)
    end
end
mvz commented 6 years ago

Thanks, @Darhazer!

bquorning commented 6 years ago

https://relishapp.com/rspec/rspec-core/v/3-7/docs/subject/explicit-subject says

We recommend using the named helper method over subject in examples.

So that would be expect(admin.something).to eq(result) in your example.

pirj commented 5 years ago

As per (https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/memoized_helpers.rb#L10):

subject was contributed by Joe Ferris to support the one-liner syntax embraced by shoulda matchers

subject's implementation is based on let, and if it's a named subject, the difference is subtle.

Semantics might be different, but in the example, it seems that the subject might equally be admin.something, not admin itself, and the use of let in this case is more correct.