Merge pull request #249 from fatkodima/multiple_assertions-handle-con…

…ditionals

Handle assertions in conditionals branches in `Minitest/MultipleAssertions` cop

changelog/change_handle_assertions_in_conditionals.md

@@ -0,0 +1 @@
+* [#249](https://github.com/rubocop/rubocop-minitest/pull/249): Handle assertions in conditionals branches in `Minitest/MultipleAssertions` cop. ([@fatkodima][])

lib/rubocop/cop/minitest/multiple_assertions.rb

@@ -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
@@ -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
 
@@ -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

lib/rubocop/cop/mixin/minitest_exploration_helpers.rb

@@ -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

test/rubocop/cop/minitest/multiple_assertions_test.rb

@@ -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
@@ -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
@@ -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
@@ -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
+        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
+        end
+      end
+    RUBY
+  end
+
   private
 
   def configure_max_assertions(max)