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

[Fix #292] ensure all kinds of assignments are correctly handled when counting assertions #293

Merged
merged 1 commit into from
Jan 6, 2024
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/fix_ensure_all_kinds_of_assignments_are.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#292](https://github.com/rubocop/rubocop-minitest/issues/292): Ensure all kinds of assignments are correctly handled when counting assertions. ([@G-Rath][])
18 changes: 14 additions & 4 deletions lib/rubocop/cop/minitest/multiple_assertions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,21 @@ def assertions_count_based_on_type(node)
end

def assertions_count_in_assignment(node)
# checking the direct expression is handled by assertion_method?
return 0 unless node.expression.block_type? || node.expression.numblock_type?
return assertions_count_based_on_type(node.expression) unless node.masgn_type?

# this will only trigger the branches for :block and :numblock type nodes
assertions_count_based_on_type(node.expression)
rhs = node.children.last

case rhs.type
when :array
rhs.children.sum { |child| assertions_count_based_on_type(child) }
when :send
assertion_method?(rhs) ? 1 : 0
else
# Play it safe and bail if we don't have any explicit handling for whatever
# the RHS type is, since at this point we're already probably dealing with
# a pretty exotic situation that's unlikely in the real world.
0
end
end
G-Rath marked this conversation as resolved.
Show resolved Hide resolved

def assertions_count_in_branches(branches)
Expand Down
201 changes: 201 additions & 0 deletions test/rubocop/cop/minitest/multiple_assertions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,49 @@ def test_asserts_once
RUBY
end

def test_does_not_crash_with_mass_assignments
assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
def test_does_not_crash
_, _ = foo
_, _ = []
_, _ = {}
_, _ = ''
_, _ = 1
end
end
RUBY
end

def test_all_types_of_assignments_are_understood
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_all_types_of_assignment
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [8/1].
# lvasgn
foo = assert_equal(1, 1)
# ivasgn
@instance_variable = assert_equal(1, 1)
# cvasgn
@@class_variable = assert_equal(1, 1)
# gvasgn
$global_variable = assert_equal(1, 1)
# casgn
# MyClass::CONSTANT_VALUE = assert_equal(1, 1)
# masgn
a, b, c = assert_equal(1, 1)
a, b, c = [assert_equal(1, 1), assert_equal(1, 1), assert_equal(1, 1)]
# op_asgn
counter += assert_equal(1, 1)
# or_asgn
result ||= assert_equal(1, 1)
# and_asgn
flag &&= assert_equal(1, 1)
end
end
RUBY
end

def test_assignments_are_not_counted_twice
assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
Expand Down Expand Up @@ -295,6 +338,22 @@ class FooTest < ActiveSupport::TestCase
RUBY
end

def test_registers_offense_when_multiple_assertions_inside_assigned_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
Expand Down Expand Up @@ -330,6 +389,27 @@ class FooTest < ActiveSupport::TestCase
RUBY
end

def test_registers_offense_when_multiple_expectations_inside_assigned_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
Expand Down Expand Up @@ -366,6 +446,27 @@ class FooTest < ActiveSupport::TestCase
RUBY
end

def test_registers_offense_when_multiple_expectations_inside_assigned_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
Expand Down Expand Up @@ -399,6 +500,26 @@ class FooTest < ActiveSupport::TestCase
RUBY
end

def test_registers_offense_when_multiple_expectations_inside_assigned_rescue
assert_offense(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [3/1].
_ = begin
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
end
RUBY
end

def test_does_not_register_offense_when_single_assertion_inside_rescue
assert_no_offenses(<<~RUBY)
class FooTest < ActiveSupport::TestCase
Expand Down Expand Up @@ -450,6 +571,86 @@ class FooTest < ActiveSupport::TestCase
RUBY
end

def test_registers_offense_when_complex_structure_with_assignments_and_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_registers_offense_when_complex_multiple_assignment_structure_and_multiple_assertions
configure_max_assertions(2)

assert_offense(<<~RUBY)
class FooTest < ActiveSupport::TestCase
test 'something' do
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [5/2].
_, _, _ = [
if condition1
assert foo
else
assert foo
end,
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,
assert(foo)
]
end
end
RUBY
end

def test_does_not_register_offense_when_complex_structure_with_single_assertion
assert_no_offenses(<<~RUBY)
class FooTest < ActiveSupport::TestCase
Expand Down