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

Handle assertions in conditionals branches in Minitest/MultipleAssertions cop #249

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/change_handle_assertions_in_conditionals.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#249](https://github.com/rubocop/rubocop-minitest/pull/249): Handle assertions in conditionals branches in `Minitest/MultipleAssertions` cop. ([@fatkodima][])
28 changes: 26 additions & 2 deletions lib/rubocop/cop/minitest/multiple_assertions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
module RuboCop
module Cop
module Minitest
# Checks if test cases contain too many assertion calls.
# Checks if test cases contain too many assertion calls. If conditional code with assertions
# is used, the branch with maximum assertions is counted.
# The maximum allowed assertion calls is configurable.
#
# @example Max: 1
Expand Down Expand Up @@ -36,7 +37,7 @@ def on_class(class_node)
return unless test_class?(class_node)

test_cases(class_node).each do |node|
assertions_count = assertions_count(node)
assertions_count = assertions_count(node.body)

next unless assertions_count > max_assertions

Expand All @@ -49,6 +50,29 @@ def on_class(class_node)

private

def assertions_count(node)
return 0 unless node.is_a?(RuboCop::AST::Node)

assertions =
case node.type
when :if, :case, :case_match
assertions_count_in_branches(node.branches)
when :rescue
assertions_count(node.body) + assertions_count_in_branches(node.branches)
when :block, :numblock
assertions_count(node.body)
else
node.each_child_node.sum { |child| assertions_count(child) }
end

assertions += 1 if assertion_method?(node)
assertions
end

def assertions_count_in_branches(branches)
branches.map { |branch| assertions_count(branch) }.max
end

def max_assertions
Integer(cop_config.fetch('Max', 3))
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/minitest_exploration_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def assertions_count(node)
end

def assertion_method?(node)
return false if !node.send_type? && !node.block_type?
return false if !node.send_type? && !node.block_type? && !node.numblock_type?

ASSERTION_PREFIXES.any? do |prefix|
method_name = node.method_name
Expand Down
230 changes: 223 additions & 7 deletions test/rubocop/cop/minitest/multiple_assertions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ def test_asserts_twice
def test_registers_offense_when_multiple_expectations_with_block
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_asserts_three_times
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [2/1].
def test_asserts_two_times
^^^^^^^^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [2/1].
assert_raises(SomeError) do
assert_equal(baz, bar)
end
Expand All @@ -32,6 +32,19 @@ def test_asserts_three_times
RUBY
end

def test_registers_offense_when_multiple_expectations_with_numblock
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_asserts_two_times
^^^^^^^^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [2/1].
assert_something do
assert_equal(_1, bar)
end
end
end
RUBY
end

def test_checks_when_inheriting_some_class_and_class_name_ending_with_test
assert_offense(<<~RUBY)
class FooTest < ActiveSupport::TestCase
Expand Down Expand Up @@ -93,20 +106,21 @@ def test_generates_a_todo_based_on_the_worst_violation
class FooTest < Minitest::Test
def test_asserts_once
assert_equal(foo, bar)
assert_equal(baz, bar)
end

def test_asserts_two_times
def test_asserts_four_times
assert_equal(foo, bar)
assert_equal(foo, bar)
assert_equal(foo, bar)
assert_equal(foo, bar)
assert_equal(baz, bar)
end
end
RUBY

assert_equal({ 'Max' => 2 }, @cop.config_to_allow_offenses[:exclude_limit])
assert_equal({ 'Max' => 4 }, @cop.config_to_allow_offenses[:exclude_limit])
end

def test_does_not_register_offense_when_multiple_expectations_in_the_test_block
def test_registers_offense_when_multiple_assertions_in_the_test_block
assert_offense(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
Expand All @@ -118,6 +132,208 @@ class FooTest < ActiveSupport::TestCase
RUBY
end

def test_registers_offense_when_multiple_assertions_inside_conditional
assert_offense(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [2/1].
if condition
assert_equal(foo, bar) # 1
assert_empty(array) # 2
else
assert_equal(foo, baz)
end
end
end
RUBY
end

def test_does_not_register_offense_when_single_assertion_inside_conditional
assert_no_offenses(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
if condition
assert_equal(foo, bar)
else
assert_equal(foo, baz)
end
end
end
RUBY
end

def test_registers_offense_when_multiple_expectations_inside_case
assert_offense(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [3/1].
case
when condition1
assert_equal(foo, bar)
assert_empty(array)
when condition2
assert_equal(foo, bar) # 1
assert_empty(array) # 2
assert_equal(foo, baz) # 3
else
assert_equal(foo, zoo)
end
end
end
RUBY
end

def test_does_not_register_offense_when_single_assertion_inside_case
assert_no_offenses(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
case
when condition1
assert_equal(foo, bar)
else
assert_equal(foo, zoo)
end
end
end
RUBY
end

def test_registers_offense_when_multiple_expectations_inside_pattern_matching
assert_offense(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [3/1].
case variable
in pattern1
assert_equal(foo, bar)
assert_empty(array)
in pattern2
assert_equal(foo, bar) # 1
assert_empty(array) # 2
assert_equal(foo, baz) # 3
else
assert_equal(foo, zoo)
end
end
end
RUBY
end

def test_does_not_register_offense_when_single_assertion_inside_pattern_matching
assert_no_offenses(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
case variable
in pattern1
assert_equal(foo, bar)
in pattern2
assert_equal(foo, baz)
end
end
end
RUBY
end

def test_registers_offense_when_multiple_expectations_inside_rescue
assert_offense(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [3/1].
do_something
rescue Foo
assert_equal(foo, bar)
assert_empty(array)
rescue Bar
assert_equal(foo, bar) # 1
assert_empty(array) # 2
assert_equal(foo, baz) # 3
end
end
RUBY
end

def test_does_not_register_offense_when_single_assertion_inside_rescue
assert_no_offenses(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
do_something
rescue Foo
assert_equal(foo, bar)
end
end
RUBY
end

def test_registers_offense_when_complex_structure_with_multiple_assertions
configure_max_assertions(2)

assert_offense(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [4/2].
if condition1
assert foo
elsif condition2
assert foo
else
begin
do_something
assert foo # 1
rescue Foo
assert foo
assert foo
rescue Bar
# noop
rescue Zoo
case
when condition
assert foo # 2
assert foo # 3
assert foo # 4
else
assert foo
assert foo
end
else
assert foo
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a source code comment to the line which line is being counted?

end
end
RUBY
end

def test_does_not_register_offense_when_complex_structure_with_single_assertion
assert_no_offenses(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
if condition1
assert foo
elsif condition2
assert foo
else
begin
do_something
rescue Foo
assert foo
rescue Bar
# noop
rescue Zoo
case
when condition
assert foo
else
assert foo
end
else
assert foo
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

end
end
RUBY
end

private

def configure_max_assertions(max)
Expand Down