Skip to content

Commit

Permalink
Rework AggregateExamples cop
Browse files Browse the repository at this point in the history
The "one expectation per example" rule has been relaxed and allows for several expectations to be set in the same example.
https://github.com/rubocop-hq/rspec-style-guide#expectations-per-example

In cases the examples don't have any setup, metadata, or even a docstring, and may be aggregated into one thus saving on sometimes expensive context setup.

The cop still does report the cases when the examples might be aggregated.

Block expectation syntax is deliberately not supported due to:
 - `subject { -> { ... } }` syntax being hard to detect
 - aggregation should use composition with `.and`
 - aggregation of the `not_to` is barely possible when a matcher doesn't
 provide a negated variant
 - aggregation of block syntax with non-block syntax should be in a
 specific order

Known caveats:
The usages if `its` that are testing private methods/readers will result in spec failure. Test only public interface!

Matchers with side effects might affects following expectations when aggregated.

Originally submitted as rubocop/rubocop-rspec#726
  • Loading branch information
pirj committed Feb 8, 2020
1 parent c01b309 commit 20931dd
Show file tree
Hide file tree
Showing 12 changed files with 1,362 additions and 450 deletions.
172 changes: 172 additions & 0 deletions lib/test_prof/cops/rspec/aggregate_examples.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
# frozen_string_literal: true

require_relative 'support/aggregate_examples/aggregation'
require_relative 'support/aggregate_examples/its'
require_relative 'support/aggregate_examples/line_range'
require_relative 'support/aggregate_examples/matchers_with_side_effects'
require_relative 'support/aggregate_examples/metadata'
require_relative 'support/aggregate_examples/node_matchers'

module RuboCop
module Cop
module RSpec
# Checks if example groups contain two or more aggregatable examples.
#
# @see https://github.com/rubocop-hq/rspec-style-guide#expectations-per-example
#
# This cop is primarily for reducing the cost of repeated expensive
# context initialization.
#
# @example
#
# # bad
# describe do
# specify do
# expect(number).to be_positive
# expect(number).to be_odd
# end
#
# it { is_expected.to be_prime }
# end
#
# # good
# describe do
# specify do
# expect(number).to be_positive
# expect(number).to be_odd
# is_expected.to be_prime
# end
# end
#
# # fair - subject has side effects
# describe do
# specify do
# expect(multiply_by(2)).to be_multiple_of(2)
# end
#
# specify do
# expect(multiply_by(3)).to be_multiple_of(3)
# end
# end
#
# Block expectation syntax is deliberately not supported due to:
#
# 1. `subject { -> { ... } }` syntax being hard to detect, e.g. the
# following looks like an example with non-block syntax, but it might
# be, depending on how the subject is defined:
#
# it { is_expected.to do_something }
#
# If the subject is defined in a `shared_context`, it's impossible to
# detect that at all.
#
# 2. Aggregation should use composition with an `.and`. Also, aggregation
# of the `not_to` expectations is barely possible when a matcher
# doesn't provide a negated variant.
#
# 3. Aggregation of block syntax with non-block syntax should be in a
# specific order.
#
# RSpec [comes with an `aggregate_failures` helper](https://relishapp.com/rspec/rspec-expectations/docs/aggregating-failures)
# not to fail the example on first unmet expectation that might come
# handy with aggregated examples.
# It can be [used in metadata form](https://relishapp.com/rspec/rspec-core/docs/expectation-framework-integration/aggregating-failures#use-%60:aggregate-failures%60-metadata),
# or [enabled globally](https://relishapp.com/rspec/rspec-core/docs/expectation-framework-integration/aggregating-failures#enable-failure-aggregation-globally-using-%60define-derived-metadata%60).
#
# @example Globally enable `aggregate_failures`
#
# # spec/spec_helper.rb
# config.define_derived_metadata do |metadata|
# unless metadata.key?(:aggregate_failures)
# metadata[:aggregate_failures] = true
# end
# end
#
# To match the style being used in the spec suite, AggregateExamples
# can be configured to add `:aggregate_failures` metadata to the
# example or not. The option not to add metadata can be also used
# when it's not desired to make expectations after previously failed
# ones, commonly known as fail-fast.
#
# The terms "aggregate examples" and "aggregate failures" not to be
# confused. The former stands for putting several expectations to
# a single example. The latter means to run all the expectations in
# the example instead of aborting on the first one.
#
# @example AddAggregateFailuresMetadata: false (default)
#
# specify do
# expect(number).to be_positive
# expect(number).to be_odd
# end
#
# @example AddAggregateFailuresMetadata: true
#
# # Metadata set using a symbol
# specify(:aggregate_failures) do
# expect(number).to be_positive
# expect(number).to be_odd
# end
#
class AggregateExamples < Cop
include Aggregation
prepend Its
include LineRange
prepend MatchersWithSideEffects
include Metadata
include NodeMatchers

MSG = 'Aggregate with the example at line %d.'.freeze

def on_block(node)
example_group_with_several_examples(node) do |all_examples|
example_clusters(all_examples).each do |_, examples|
examples[1..-1].each do |example|
add_offense(example,
location: :expression,
message: message_for(example, examples[0]))
end
end
end
end

def autocorrect(example_node)
clusters = example_clusters_for_autocorrect(example_node)
return if clusters.empty?

lambda do |corrector|
clusters.each do |metadata, examples|
range = range_for_replace(examples)
replacement = aggregated_example(examples, metadata)
corrector.replace(range, replacement)
examples[1..-1].map { |example| drop_example(corrector, example) }
end
end
end

private

# Clusters of examples in the same example group, on the same nesting
# level that can be aggregated.
def example_clusters(all_examples)
all_examples
.select { |example| example_with_expectations_only?(example) }
.group_by { |example| metadata_without_aggregate_failures(example) }
.select { |_, examples| examples.count > 1 }
end

# Clusters of examples that can be aggregated without losing any
# information (e.g. metadata or docstrings)
def example_clusters_for_autocorrect(example_node)
examples_in_group = example_node.parent.each_child_node(:block)
.select { |example| example_for_autocorrect?(example) }
example_clusters(examples_in_group)
end

def message_for(_example, first_example)
MSG % first_example.loc.line
end
end
end
end
end
53 changes: 53 additions & 0 deletions lib/test_prof/cops/rspec/aggregate_examples/aggregation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
module RuboCop
module Cop
module RSpec
class AggregateExamples < Cop
# @internal
# Aggregation helpers.
module Aggregation
private

def drop_example(corrector, example)
aggregated_range = range_by_whole_lines(example.source_range,
include_final_newline: true)
corrector.remove(aggregated_range)
end

def range_for_replace(examples)
range = range_by_whole_lines(examples.first.source_range,
include_final_newline: true)
next_range = range_by_whole_lines(examples[1].source_range)
if adjacent?(range, next_range)
range.resize(range.length + 1)
else
range
end
end

def adjacent?(range, another_range)
range.end_pos + 1 == another_range.begin_pos
end

def aggregated_example(examples, metadata)
base_indent = ' ' * examples.first.source_range.column
metadata = metadata_for_aggregated_example(metadata)
[
"#{base_indent}specify#{metadata} do",
*examples.map { |example| transform_body(example, base_indent) },
"#{base_indent}end\n"
].join("\n")
end

# Extracts and transforms the body, keeping proper indentation.
def transform_body(node, base_indent)
"#{base_indent} #{new_body(node)}"
end

def new_body(node)
node.body.source
end
end
end
end
end
end
89 changes: 89 additions & 0 deletions lib/test_prof/cops/rspec/aggregate_examples/its.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
module RuboCop
module Cop
module RSpec
class AggregateExamples < Cop
# @example `its`
#
# # Supports regular `its` call with an attribute/method name,
# # or a chain of methods expressed as a string with dots.
#
# its(:one) { is_expected.to be(true) }
# its('two') { is_expected.to be(false) }
# its('phone_numbers.size') { is_expected.to be 2 }
#
# @example `its` with single-element array argument
#
# # Also supports single-element array argument.
#
# its(['headers']) { is_expected.to include(encoding: 'text') }
#
# @example `its` with multi-element array argument is ambiguous
#
# # Does not support `its` with multi-element array argument due to
# # an ambiguity. Transformation depends on the type of the subject:
# # - a Hash: `hash[element1][element2]...`
# # - and arbitrary type: `hash[element1, element2, ...]`
# # It is impossible to infer the type to propose a proper correction.
#
# its(['ambiguous', 'elements']) { ... }
#
# @example `its` with metadata
#
# its('header', html: true) { is_expected.to include(text: 'hello') }
#
module Its
extend RuboCop::NodePattern::Macros

private

# It's impossible to aggregate `its` body as is, it needs to be
# converted to `expect(subject.something).to ...`
def new_body(node)
return super unless its?(node)

transform_its(node.body, node.send_node.arguments)
end

def transform_its(body, arguments)
argument = arguments.first
replacement = case argument.type
when :array
key = argument.values.first
"expect(subject[#{key.source}])"
else
property = argument.value
"expect(subject.#{property})"
end
body.source.gsub(/is_expected|are_expected/, replacement)
end

def example_metadata(example)
return super unless its?(example.send_node)

# First parameter to `its` is not metadata.
example.send_node.arguments[1..-1]
end

def its?(node)
node.method_name == :its
end

# In addition to base definition, matches examples with:
# - no `its` with an multiple-element array argument due to
# an ambiguity, when SUT can be a hash, and result will be defined
# by calling `[]` on SUT subsequently, e.g. `subject[one][two]`,
# or any other type of object implementing `[]`, and then all the
# array arguments are passed to `[]`, e.g. `subject[one, two]`.
def_node_matcher :example_for_autocorrect?, <<-PATTERN
[ #super !#its_with_multiple_element_array_argument? ]
PATTERN

def_node_matcher :its_with_multiple_element_array_argument?,
<<-PATTERN
(block (send nil? :its (array _ _ ...)) ...)
PATTERN
end
end
end
end
end
29 changes: 29 additions & 0 deletions lib/test_prof/cops/rspec/aggregate_examples/line_range.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
module RuboCop
module Cop
module RSpec
class AggregateExamples < Cop
# @internal Support methods for keeping newlines around examples.
module LineRange
include RangeHelp

private

def range_for_replace(examples)
range = range_by_whole_lines(examples.first.source_range,
include_final_newline: true)
next_range = range_by_whole_lines(examples[1].source_range)
if adjacent?(range, next_range)
range.resize(range.length + 1)
else
range
end
end

def adjacent?(range, another_range)
range.end_pos + 1 == another_range.begin_pos
end
end
end
end
end
end
Loading

0 comments on commit 20931dd

Please sign in to comment.