Fix specs for code transparently fixed with RuboCop 0.84 update #914
Conversation
| expect_offense(<<-RUBY) | ||
| it do | ||
| foo = double | ||
| ^^^^^^ Prefer using verifying doubles over normal doubles. |
There was a problem hiding this comment.
It's quite strange how it worked like that, since IgnoreNameless: true by default.
Nice, it's fixed now.
This gives me an interesting thought, since on the one hand, it's a bug fix, and on the other, it's a change in behaviour. But we're unable to "protect" our users from that change.
There was a problem hiding this comment.
Perhaps cop_config was not merged with the default values?
Anyway, it's better to have the values explicitly set in specs and not rely on defaults
There was a problem hiding this comment.
Exactly. Previously – before rubocop/rubocop#7970 – the default configuration was not present when testing. And it is now, so we can simplify a lot of our test.
I was too working on fixing our specs (and applied the exact same changes to spec/rubocop/cop/rspec/verified_doubles_spec.rb) and have taken the liberty of adding a few commits to your branch. I hope that is ok with you @pirj
There was a problem hiding this comment.
This is such a wonderful idea. I recall it was discussed before, and so good it's now in place.
is ok with you
Of course, nice cleanup!
There was a problem hiding this comment.
I mention in the commit message of 4e7272b that the change in RuboCop actually introduces a “mystery guest”, which could be considered a code smell.
There was a problem hiding this comment.
Everything works and looks nice. Do you have something else in mind tbd here, or is it good to merge?
There was a problem hiding this comment.
I think we‘re good to merge.
| expect_offense(<<-RUBY) | ||
| it do | ||
| foo = double | ||
| ^^^^^^ Prefer using verifying doubles over normal doubles. |
There was a problem hiding this comment.
Perhaps cop_config was not merged with the default values?
Anyway, it's better to have the values explicitly set in specs and not rely on defaults
After rubocop/rubocop#7970, all specs with the `:config` metadata by default use the default configuration. So now, there is often no reason to add a cop's default configuration in specs.
In rubocop/rubocop#7970 the shared context `config` was updated to include the following: let(:cop) do cop_class.new(config, cop_options) .tap { |cop| cop.processed_source = processed_source } end This is functionally equivalent to what we have all over our specs: subject(:cop) { described_class.new(config) } The downside is that our specs become a less explicit. Actually they now have a mystery guest, which may be considered a smell.
There is no longer by default a "when configuration is empty" situation
to test. We always have the default configuration when testing.
The existing tests ("should only fail" -> "onlies fail") did not add
much value.
It seems that RuboCop comes with certain fixes that improve how RuboCop RSpec itself works!
Before submitting the PR make sure the following are checked:
master(if not - rebase it).bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).