Skip to content

Commit

Permalink
Enable MultipleMemoizedHelpers by default
Browse files Browse the repository at this point in the history 
Switching the cop on by default to match RSpec recommendations from its
docs and the community RSpec Style Guide recommendations.

    `let` _can_ enhance readability when used sparingly (1,2, or
    maybe 3 declarations) in any given example group, but that can
    quickly degrade with overuse. YMMV.

https://github.com/rspec/rspec-core/blob/5d87c28bb828ba14b54a0d8cfddf044894009314/lib/rspec/core/memoized_helpers.rb#L257

https://reddit.com/r/ruby/comments/73nrhk/ama_the_authors_of_effective_testing_with_rspec_3/dnsyanp/

https://rspec.rubystyle.guide/#let-blocks

Example change to dramatically reduce the number of used memoized
helpers without resorting to methods rubocop/rubocop#8447

There is no limit to the number of memoized helpers being used in
practice:

10 lets https://github.com/forem/forem/blob/master/spec/support/api_analytics_shared_examples.rb#L2
20 lets https://github.com/chef/chef/blob/master/spec/unit/provider/systemd_unit_spec.rb#L23
30 lets https://github.com/chef/chef/blob/master/spec/unit/policy_builder/policyfile_spec.rb#L24
40 lets https://github.com/diaspora/diaspora/blob/develop/spec/integration/migration_service_spec.rb#L7
50 lets https://github.com/chef/chef/blob/master/spec/functional/resource/dsc_script_spec.rb#L71

Allowing `subject` by default, i.e. subjects don't count towards the
number of memoized helpers, since there is a dedicated
`RSpec/MultipleSubjects` cop. Combination with it provides infinite
possibilities to tweak the maximum allowed number of memoized helpers to
meet the project style.
  • Loading branch information
@pirj
pirj committed Aug 7, 2020
1 parent a602496 commit caf0f48
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 83 deletions.
4 changes: 2 additions & 2 deletions config/default.yml
View file
Expand Up @@ -396,8 +396,8 @@ RSpec/MultipleExpectations:

RSpec/MultipleMemoizedHelpers:
Description: Checks if example groups contain too many `let` and `subject` calls.
Enabled: false
AllowSubject: false
Enabled: true
AllowSubject: true
Max: 5
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleMemoizedHelpers

Expand Down
20 changes: 8 additions & 12 deletions lib/rubocop/cop/rspec/multiple_memoized_helpers.rb
View file
Expand Up @@ -17,7 +17,7 @@ module RSpec
# let!(:baz) { [] }
# let(:qux) { [] }
# let(:quux) { [] }
# subject(:quuz) { {} }
# let(:quuz) { {} }
# end
#
# describe MyClass do
Expand All @@ -28,7 +28,7 @@ module RSpec
# context 'when stuff' do
# let(:qux) { [] }
# let(:quux) { [] }
# subject(:quuz) { {} }
# let(:quuz) { {} }
# end
# end
#
Expand All @@ -38,7 +38,7 @@ module RSpec
# let!(:baz) { [] }
# let(:qux) { [] }
# let(:quux) { [] }
# subject(:quuz) { {} }
# let(:quuz) { {} }
# end
#
# describe MyClass do
Expand All @@ -51,17 +51,17 @@ module RSpec
# context 'when other stuff' do
# let(:qux) { [] }
# let(:quux) { [] }
# subject(:quuz) { {} }
# let(:quuz) { {} }
# end
# end
#
# @example with AllowSubject configuration
#
# # rubocop.yml
# # RSpec/MultipleMemoizedHelpers:
# # AllowSubject: true
# # AllowSubject: false
#
# # good
# # bad - `subject` counts towards memoized helpers
# describe MyClass do
# let(:foo) { [] }
# let(:bar) { [] }
Expand All @@ -75,16 +75,12 @@ module RSpec
#
# # rubocop.yml
# # RSpec/MultipleMemoizedHelpers:
# # Max: 0
# # Max: 1
#
# # bad
# describe MyClass do
# let(:foo) { [] }
# end
#
# # good
# describe MyClass do
# def foo; []; end
# let(:bar) { [] }
# end
#
class MultipleMemoizedHelpers < Base
Expand Down
24 changes: 10 additions & 14 deletions manual/cops_rspec.md
View file
Expand Up @@ -2123,7 +2123,7 @@ Max | `1` | Integer

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Disabled | Yes | No | - | -
Enabled | Yes | No | - | -

Checks if example groups contain too many `let` and `subject` calls.

Expand All @@ -2141,7 +2141,7 @@ describe MyClass do
let!(:baz) { [] }
let(:qux) { [] }
let(:quux) { [] }
subject(:quuz) { {} }
let(:quuz) { {} }
end

describe MyClass do
Expand All @@ -2152,7 +2152,7 @@ describe MyClass do
context 'when stuff' do
let(:qux) { [] }
let(:quux) { [] }
subject(:quuz) { {} }
let(:quuz) { {} }
end
end

Expand All @@ -2162,7 +2162,7 @@ describe MyClass do
let!(:baz) { [] }
let(:qux) { [] }
let(:quux) { [] }
subject(:quuz) { {} }
let(:quuz) { {} }
end

describe MyClass do
Expand All @@ -2175,7 +2175,7 @@ describe MyClass do
context 'when other stuff' do
let(:qux) { [] }
let(:quux) { [] }
subject(:quuz) { {} }
let(:quuz) { {} }
end
end
```
Expand All @@ -2184,9 +2184,9 @@ end
```ruby
# rubocop.yml
# RSpec/MultipleMemoizedHelpers:
# AllowSubject: true
# AllowSubject: false

# good
# bad - `subject` counts towards memoized helpers
describe MyClass do
let(:foo) { [] }
let(:bar) { [] }
Expand All @@ -2201,24 +2201,20 @@ end
```ruby
# rubocop.yml
# RSpec/MultipleMemoizedHelpers:
# Max: 0
# Max: 1

# bad
describe MyClass do
let(:foo) { [] }
end

# good
describe MyClass do
def foo; []; end
let(:bar) { [] }
end
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
AllowSubject | `false` | Boolean
AllowSubject | `true` | Boolean
Max | `5` | Integer

### References
Expand Down
104 changes: 49 additions & 55 deletions spec/rubocop/cop/rspec/multiple_memoized_helpers_spec.rb
View file
Expand Up @@ -3,7 +3,7 @@
RSpec.describe RuboCop::Cop::RSpec::MultipleMemoizedHelpers, :config do
let(:cop_config) { { 'Max' => 1 } }

it 'flags an offense when using more than max `#let` calls' do
it 'flags excessive `#let`' do
expect_offense(<<~RUBY)
describe Foo do
^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1]
Expand All @@ -13,83 +13,66 @@
RUBY
end

it 'flags an offense when using `#subject` without name' do
it 'flags excessive `#let!`' do
expect_offense(<<~RUBY)
describe Foo do
^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1]
subject { Foo.new }
let(:foo) { Foo.new }
let!(:bar) { Bar.new }
end
RUBY
end

it 'flags an offense when using `#subject` with name' do
expect_offense(<<~RUBY)
describe Foo do
^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1]
subject(:foo) { Foo.new }
let(:bar) { Foo.new }
end
RUBY
end

it 'flags an offense when using `#let!`' do
expect_offense(<<~RUBY)
it 'ignores a reasonable number of memoized helpers' do
expect_no_offenses(<<~RUBY)
describe Foo do
^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1]
let(:foo) { Foo.new }
let!(:bar) { Foo.new }
end
RUBY
end

it 'flags an offense when using `#subject!`' do
expect_offense(<<~RUBY)
it 'ignores overridden `#let`' do
expect_no_offenses(<<~RUBY)
describe Foo do
^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1]
subject!(:foo) { Foo.new }
let(:bar) { Foo.new }
let(:foo) { Foo.new }
context do
let(:foo) { Foo.new(1) }
context do
let(:foo) { Foo.new(1, 2) }
end
end
end
RUBY
end

it 'does not flag an offense when using <= max `#let` calls' do
it 'ignores `#subject`' do
expect_no_offenses(<<~RUBY)
describe Foo do
subject(:bar) { Bar.new }
let(:foo) { Foo.new }
end
RUBY
end

it 'ignores overridden variables' do
it 'ignores `#subject!`' do
expect_no_offenses(<<~RUBY)
describe Foo do
subject!(:bar) { Bar.new }
let(:foo) { Foo.new }
context do
let(:foo) { Foo.new(1) }
context do
let(:foo) { Foo.new(1, 2) }
end
end
end
RUBY
end

it 'ignores overridden nameless subjects' do
it 'ignores `#subject` without a name' do
expect_no_offenses(<<~RUBY)
describe Foo do
subject { Foo.new }
context do
subject { Foo.new(1) }
context do
subject { Foo.new(1, 2) }
end
end
let(:bar) { Bar.new }
end
RUBY
end

it 'flags an offense when too many `#let` calls are nested' do
it 'flags nested `#let`' do
expect_offense(<<~RUBY)
describe Foo do
let(:foo) { Foo.new }
Expand All @@ -102,7 +85,7 @@
RUBY
end

it 'does not flag an offense when `#let` calls are distributed' do
it 'ignores distributed `#let`' do
expect_no_offenses(<<~RUBY)
describe Foo do
context 'when bloo' do
Expand All @@ -116,38 +99,49 @@
RUBY
end

it 'does not flag an offense when using `#before`' do
expect_no_offenses(<<~RUBY)
describe Foo do
before { foo }
end
RUBY
end

context 'when using AllowSubject configuration', :config do
let(:cop_config) { { 'Max' => 0, 'AllowSubject' => true } }
let(:cop_config) { { 'Max' => 1, 'AllowSubject' => false } }

it 'flags an offense when using `#let`' do
it 'flags `#subject` without name' do
expect_offense(<<~RUBY)
describe Foo do
^^^^^^^^^^^^^^^ Example group has too many memoized helpers [1/0]
^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1]
subject { Foo.new }
let(:foo) { Foo.new }
end
RUBY
end

it 'does not flag an offense when using `#subject` without a name' do
expect_no_offenses(<<~RUBY)
it 'flags `#subject`' do
expect_offense(<<~RUBY)
describe Foo do
subject { Foo.new }
^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1]
subject(:foo) { Foo.new }
let(:bar) { Foo.new }
end
RUBY
end

it 'does not flag an offense when using `#subject` with a name' do
it 'flags `#subject!`' do
expect_offense(<<~RUBY)
describe Foo do
^^^^^^^^^^^^^^^ Example group has too many memoized helpers [2/1]
subject!(:foo) { Foo.new }
let(:bar) { Foo.new }
end
RUBY
end

it 'ignores overridden subjects' do
expect_no_offenses(<<~RUBY)
describe Foo do
subject(:foo) { Foo.new }
subject { Foo.new }
context do
subject { Foo.new(1) }
context do
subject { Foo.new(1, 2) }
end
end
end
RUBY
end
Expand Down

0 comments on commit caf0f48

Please sign in to comment.