rubocop / rubocop-rspec

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

RSpec/UnspecifiedException triggers for `not_to` within a `to` block #1954

Closed G-Rath closed 1 month ago

G-Rath commented 2 months ago

Given the following:

expect do
  expect { described_class.new(report).populate! }.not_to raise_error
end.to change(ReportTicket, :count).by(3)

Before #1940 this was fine, but now it's being flagged as an error - if I replace the outer to with not_to, it passes again.

pirj commented 2 months ago

Walking up the tree seems incorrect here. I recall making a very similar note in another issue here, and the fix is the same.

@rubocop/rubocop-rspec why do we have this cop? Doesn’t it fall into a non-goal category considering this RSpec built-in warning?

Should we fix the cop, or deprecate it?

bquorning commented 2 months ago

It looks like that feature was added in RSpec 3.3, over 9 years ago. I’d say we should deprecate this cop.

ydah commented 2 months ago

It might still have been worthwhile to have this cop if it had supported autocorrection, but since it does not support autocorrection, this cop should be deprecated.

G-Rath commented 2 months ago

hmm so on second thought, I'm less in favor of deprecating this cop now because it seems RSpec doesn't fail when printing this warning - it might be possible to configure RSpec to error, but overall it does seem slightly useful to be able to lint this in parallel with running your whole test suite.

It also sounds like there's someone working to address this - I assume that while the team have expressed a wish to deprecate the rule, they still wouldn't outright reject a pull request fixing the underlying issue so I might just for now add an inline disable in my project for now and see how this unfolds in the community