Bug: false positive of RSpec/LetSetup with context and outer shared_examples

[dmytro-savochkin]

This issue produces false positives of Rspec/LetSetup with the latest rubocop-rspec (1.36.0) when we put let! inside some context and we also have shared_examples referencing that let! outside the context.

Something like this:

shared_examples 'some shared example' do
  it 'checks something' do
    is_expected.to eq user
  end
end

context 'when inside some context' do
  let!(:user) { create :user }

  include_examples 'some shared example'
end

If we are to run rubocop-rspec on the code above it would produce the following:

spec/test_spec.rb:8:3: C: RSpec/LetSetup: Do not use let! to setup objects not referenced in tests.
  let!(:user) { create :user }
  ^^^^^^^^^^^

1 file inspected, 1 offense detected

[Darhazer]

This is a limitation of the static analysis. The shared examples could be defined in a separate file as well, and Rubocop does not cross-reference files. I would suggest to explicitly pass user in this well, like

include_examples 'some shared example', user

and

shared_examples 'some shared example' do |user|

This way you will not have a hidden dependency in the code.

[typhoon2099]

I have just tried the proposed solution and it causes failures in my tests, as the user created by FactoryBot is reused between tests (since it is created outside of a test context). You also cannot declare user in a let above the include_examples and use it as a parameter for the same reason.

[pirj]

How about

  include_examples 'some shared example' do
    let!(:user) { create :user }
  end

[typhoon2099]

That works, but it still raises a RSpec/LetSetup warning.

[Darhazer]

I guess the cop should really ignore let! inside include_examples blocks, as it can't determine whether the let is used in the shared examples or not

[pirj]

Agree. Another exception would be an example group that is including examples, since it's impossible to determine if it's referenced or not.

[StuartHadfield]

I'm fairly certain this isn't limited to shared examples.

If you have a top-level subject and create an object inside a context with a let!, you will get the same false positive.

I.e.

subject(:foo) do
  post "url",
    params: {
      id: my_object.id
    }
end

context "when calling the method" do
  let!(:my_object) { FactoryBot.create(object) } # you will get a false positive here
  
  it { .. }
end

[njm506]

I've hit this when using let! in shared_context, which is the reverse situation.

[pirj]

@StuartHadfield Why would you use a bang-let over a regular let, if the variable is referenced from a top-level subject definition, i.e. is always initialized?

@njm506 Can you please describe your case in more detail?

[njm506]

We use let!, for example, to set something up that needs to be present before before is executed, and that we also need to reference in the tests. Here's a mock-up of part of the spec in question:

  shared_context 'when there is a local record' do
    let!(:record) do # rubocop:disable RSpec/LetSetup
      test_record(
        key: 'value' # Our use-case has a lot more properties
      )
    end
  end

  context 'when syncing a new foo' do
    include_context 'when there is a local record'

    before do
      rest_api_post.to_return(body: '{"id":"new-id"}', status: 200)
      described_class.sync_all	# This references the record that test_record created
    end

    it 'creates the foo' do
      expect(rest_api_post.with do |req|
        # Some criteria here
      end).to have_been_requested.once
    end

    it 'stores the returned id' do
      expect(record.foo_id).to eq('new-id')
    end
  end

  context 'when syncing a new foo but creation fails' do
    include_context 'when there is a local record'

    before do
      rest_api_post.to_return(body: '', status: 400)
      described_class.sync_all
    end

    ...
  end

[pirj]

I don't receive any offences @njm506

Inspecting 1 file
C

Offenses:

spec/a_spec.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
RSpec.describe 'A' do
^
spec/a_spec.rb:15:81: C: Layout/LineLength: Line is too long. [84/80]
      described_class.sync_all	# This references the record that test_record created
                                                                                ^^^^
spec/a_spec.rb:36:1: C: Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end.

1 file inspected, 3 offenses detected

[njm506]

I'm using 1.42.0 and it's giving me it for that example (when I remove the rubocop:disable marker):

Inspecting 1 file
C

Offenses:

spec/test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
describe 'foo' do
^
spec/test.rb:3:5: C: RSpec/LetSetup: Do not use let! to setup objects not referenced in tests. (https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/LetSetup)
    let!(:record) do
    ^^^^^^^^^^^^^

1 file inspected, 2 offenses detected

[phorsuedzie]

Trying to add 2 cowries

Why would you use a bang-let over a regular let, if the variable is referenced from a top-level subject definition, i.e. is always initialized?

The plain let is lazy evaluated, i.e. executed when first called. If the subject is the first to instantiate the fixture, all of the instantiation's side-effects creep into the subject's execution scope. E.g. consider the example is about to measure the number of database calls for the execution of subject. This would include all database statements to instantiate the fixture too, likely unwanted.

let! is evaluated clearly before the subject is executed, and can therefore help to keep all unwanted, setup related side-effects away from the subject.

PS: The alternative way to keep side-effects out of the subject is a @fixture_value_from_instance_variable. But this has the disadvantage of an "order dependency" between assignment and access, and it also lacks "there is exactly one value visible to the example" (let: the most inner one to define the value, @fixture: whatever is assigned at the time of access, could be different from the finally assigned value if there are later assignments).

[phorsuedzie]

It's not actually about shared examples:

RSpec.describe User do
  let!(:user) { create :user }

  describe "#update with a name" do
    subject(:update_user) { user.update(name:) }

    context "with present name" do
      let!(:name) { short_string }
  
      it "works" do
        expect { update_user }.to change {
          User.find(user.id).name
        }.to(name)
      end
    end

    context "with empty name" do
       let!(:name) { "" } # rubocop:disable RSpec/LetSetup

       it "fails" do
         expect { update_user }.to(raise_error)
       end
     end
   end
end