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

RSpec/RepeatedSubjectCall flags a false positive #1821

Closed
luizkowalski opened this issue Mar 1, 2024 · 4 comments · Fixed by #1822
Closed

RSpec/RepeatedSubjectCall flags a false positive #1821

luizkowalski opened this issue Mar 1, 2024 · 4 comments · Fixed by #1822
Labels

Comments

@luizkowalski
Copy link

Hey 👋🏻

I have a test like this:

subject { described_class.new(url) }
...
context "remaining is exhausted" do
  let(:remaining) { 0 }

  it "fails on the next call" do
    expect { subject.call }.not_to raise_error
    expect { subject.call }.to raise_error(RateLimiter::LimitedError)
  end
end
...

from what I understand, subject is memorized; in this case, described_class.new(url) but the .call is not memoized. I know that because this test is currently passing.

I don't think this cop should flag such cases (when the subject invokes another method).

@franzliedke
Copy link
Contributor

Ha, I just came here to report the same problem.

Another example with false positives:

expect { subject.method1 }.to raise_error(SomeException)
expect { subject.method2 }.to raise_error(SomeException)
expect { subject.method3 }.to raise_error(SomeException)

@tobiasamft
Copy link

Hello 👋 , I just wanted to add another example which could be worth mentioning:

class Something
  def work
    private_method
  end

  private

  def private_method
    # some call that may raise an error
  end
end
describe Something do
  subject { described_class.new }

  describe '#work' do
    it 'raises' do
      allow(subject).to receive(:private_method).and_raise(
        StandardError,
        'error message',
      )
      expect { subject.work }.to raise_error(StandardError, 'error message')
    end
  end
end

ydah added a commit that referenced this issue Mar 1, 2024
@ydah ydah closed this as completed in #1822 Mar 1, 2024
danidoni added a commit to openSUSE/open-build-service that referenced this issue Mar 4, 2024
The RSpec/RepeatedSubjectCall is givin false positives on, for example:

- spec/models/workflow/step/branch_package_step_spec.rb:597
- spec/models/workflow/step/branch_package_step_spec.rb:608

See rubocop/rubocop-rspec#1821 for the details.
@sgessa
Copy link

sgessa commented Aug 24, 2024

Not sure if this case is already covered but seems a false positive to me.
The following example is called from a shared example.

it 'raises an error if country GB' do
  if country_id == 'GB'
    expect { action }.to raise_error(ArgumentError)
  else
    expect { action }.not_to raise_error
  end
end

@pirj
Copy link
Member

pirj commented Aug 24, 2024

The expectation inside the "else" clause doesn’t match the docstring. It deserves to be extracted to a separate co text or example.
Thus, no need for an if and no false positive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants