Rubocop-rspec: 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