rubocop-rspec icon indicating copy to clipboard operation
rubocop-rspec copied to clipboard

Cop idea: detect let that have to be a subject

Open Darhazer opened this issue 7 years ago • 5 comments

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

Darhazer avatar Apr 23 '17 15:04 Darhazer

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

mvz avatar Mar 09 '18 09:03 mvz

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

Darhazer avatar Mar 09 '18 09:03 Darhazer

Thanks, @Darhazer!

mvz avatar Mar 09 '18 09:03 mvz

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.

bquorning avatar Mar 09 '18 20:03 bquorning

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.

pirj avatar Aug 20 '19 10:08 pirj