Skip to content

Commit 1cca1e0

Browse files
koickoic
authored
Merge pull request #293 from G-Rath/check-for-expression
[Fix #292] ensure all kinds of assignments are correctly handled when counting assertions
2 parents 9a7a882 + 916ffbc commit 1cca1e0

File tree

3 files changed

+216
-4
lines changed

3 files changed

+216
-4
lines changed

‎changelog/fix_ensure_all_kinds_of_assignments_are.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#292](https://github.com/rubocop/rubocop-minitest/issues/292): Ensure all kinds of assignments are correctly handled when counting assertions. ([@G-Rath][])

‎lib/rubocop/cop/minitest/multiple_assertions.rb

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,21 @@ def assertions_count_based_on_type(node)
7474
end
7575

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

80-
# this will only trigger the branches for :block and :numblock type nodes
81-
assertions_count_based_on_type(node.expression)
79+
rhs = node.children.last
80+
81+
case rhs.type
82+
when :array
83+
rhs.children.sum { |child| assertions_count_based_on_type(child) }
84+
when :send
85+
assertion_method?(rhs) ? 1 : 0
86+
else
87+
# Play it safe and bail if we don't have any explicit handling for whatever
88+
# the RHS type is, since at this point we're already probably dealing with
89+
# a pretty exotic situation that's unlikely in the real world.
90+
0
91+
end
8292
end
8393

8494
def assertions_count_in_branches(branches)

‎test/rubocop/cop/minitest/multiple_assertions_test.rb

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,49 @@ def test_asserts_once
113113
RUBY
114114
end
115115

116+
def test_does_not_crash_with_mass_assignments
117+
assert_no_offenses(<<~RUBY)
118+
class FooTest < Minitest::Test
119+
def test_does_not_crash
120+
_, _ = foo
121+
_, _ = []
122+
_, _ = {}
123+
_, _ = ''
124+
_, _ = 1
125+
end
126+
end
127+
RUBY
128+
end
129+
130+
def test_all_types_of_assignments_are_understood
131+
assert_offense(<<~RUBY)
132+
class FooTest < Minitest::Test
133+
def test_all_types_of_assignment
134+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [8/1].
135+
# lvasgn
136+
foo = assert_equal(1, 1)
137+
# ivasgn
138+
@instance_variable = assert_equal(1, 1)
139+
# cvasgn
140+
@@class_variable = assert_equal(1, 1)
141+
# gvasgn
142+
$global_variable = assert_equal(1, 1)
143+
# casgn
144+
# MyClass::CONSTANT_VALUE = assert_equal(1, 1)
145+
# masgn
146+
a, b, c = assert_equal(1, 1)
147+
a, b, c = [assert_equal(1, 1), assert_equal(1, 1), assert_equal(1, 1)]
148+
# op_asgn
149+
counter += assert_equal(1, 1)
150+
# or_asgn
151+
result ||= assert_equal(1, 1)
152+
# and_asgn
153+
flag &&= assert_equal(1, 1)
154+
end
155+
end
156+
RUBY
157+
end
158+
116159
def test_assignments_are_not_counted_twice
117160
assert_no_offenses(<<~RUBY)
118161
class FooTest < Minitest::Test
@@ -295,6 +338,22 @@ class FooTest < ActiveSupport::TestCase
295338
RUBY
296339
end
297340

341+
def test_registers_offense_when_multiple_assertions_inside_assigned_conditional
342+
assert_offense(<<~RUBY)
343+
class FooTest < ActiveSupport::TestCase
344+
test 'something' do
345+
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [2/1].
346+
_ = if condition
347+
assert_equal(foo, bar) # 1
348+
assert_empty(array) # 2
349+
else
350+
assert_equal(foo, baz)
351+
end
352+
end
353+
end
354+
RUBY
355+
end
356+
298357
def test_does_not_register_offense_when_single_assertion_inside_conditional
299358
assert_no_offenses(<<~RUBY)
300359
class FooTest < ActiveSupport::TestCase
@@ -330,6 +389,27 @@ class FooTest < ActiveSupport::TestCase
330389
RUBY
331390
end
332391

392+
def test_registers_offense_when_multiple_expectations_inside_assigned_case
393+
assert_offense(<<~RUBY)
394+
class FooTest < ActiveSupport::TestCase
395+
test 'something' do
396+
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [3/1].
397+
_ = case
398+
when condition1
399+
assert_equal(foo, bar)
400+
assert_empty(array)
401+
when condition2
402+
assert_equal(foo, bar) # 1
403+
assert_empty(array) # 2
404+
assert_equal(foo, baz) # 3
405+
else
406+
assert_equal(foo, zoo)
407+
end
408+
end
409+
end
410+
RUBY
411+
end
412+
333413
def test_does_not_register_offense_when_single_assertion_inside_case
334414
assert_no_offenses(<<~RUBY)
335415
class FooTest < ActiveSupport::TestCase
@@ -366,6 +446,27 @@ class FooTest < ActiveSupport::TestCase
366446
RUBY
367447
end
368448

449+
def test_registers_offense_when_multiple_expectations_inside_assigned_pattern_matching
450+
assert_offense(<<~RUBY)
451+
class FooTest < ActiveSupport::TestCase
452+
test 'something' do
453+
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [3/1].
454+
_ = case variable
455+
in pattern1
456+
assert_equal(foo, bar)
457+
assert_empty(array)
458+
in pattern2
459+
assert_equal(foo, bar) # 1
460+
assert_empty(array) # 2
461+
assert_equal(foo, baz) # 3
462+
else
463+
assert_equal(foo, zoo)
464+
end
465+
end
466+
end
467+
RUBY
468+
end
469+
369470
def test_does_not_register_offense_when_single_assertion_inside_pattern_matching
370471
assert_no_offenses(<<~RUBY)
371472
class FooTest < ActiveSupport::TestCase
@@ -399,6 +500,26 @@ class FooTest < ActiveSupport::TestCase
399500
RUBY
400501
end
401502

503+
def test_registers_offense_when_multiple_expectations_inside_assigned_rescue
504+
assert_offense(<<~RUBY)
505+
class FooTest < ActiveSupport::TestCase
506+
test 'something' do
507+
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [3/1].
508+
_ = begin
509+
do_something
510+
rescue Foo
511+
assert_equal(foo, bar)
512+
assert_empty(array)
513+
rescue Bar
514+
assert_equal(foo, bar) # 1
515+
assert_empty(array) # 2
516+
assert_equal(foo, baz) # 3
517+
end
518+
end
519+
end
520+
RUBY
521+
end
522+
402523
def test_does_not_register_offense_when_single_assertion_inside_rescue
403524
assert_no_offenses(<<~RUBY)
404525
class FooTest < ActiveSupport::TestCase
@@ -450,6 +571,86 @@ class FooTest < ActiveSupport::TestCase
450571
RUBY
451572
end
452573

574+
def test_registers_offense_when_complex_structure_with_assignments_and_multiple_assertions
575+
configure_max_assertions(2)
576+
577+
assert_offense(<<~RUBY)
578+
class FooTest < ActiveSupport::TestCase
579+
test 'something' do
580+
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [4/2].
581+
_= if condition1
582+
assert foo
583+
elsif condition2
584+
assert foo
585+
else
586+
begin
587+
do_something
588+
assert foo # 1
589+
rescue Foo
590+
assert foo
591+
assert foo
592+
rescue Bar
593+
# noop
594+
rescue Zoo
595+
_ = case
596+
when condition
597+
assert foo # 2
598+
assert foo # 3
599+
assert foo # 4
600+
else
601+
assert foo
602+
assert foo
603+
end
604+
else
605+
assert foo
606+
end
607+
end
608+
end
609+
end
610+
RUBY
611+
end
612+
613+
def test_registers_offense_when_complex_multiple_assignment_structure_and_multiple_assertions
614+
configure_max_assertions(2)
615+
616+
assert_offense(<<~RUBY)
617+
class FooTest < ActiveSupport::TestCase
618+
test 'something' do
619+
^^^^^^^^^^^^^^^^^^^ Test case has too many assertions [5/2].
620+
_, _, _ = [
621+
if condition1
622+
assert foo
623+
else
624+
assert foo
625+
end,
626+
begin
627+
do_something
628+
assert foo # 1
629+
rescue Foo
630+
assert foo
631+
assert foo
632+
rescue Bar
633+
# noop
634+
rescue Zoo
635+
_ = case
636+
when condition
637+
assert foo # 2
638+
assert foo # 3
639+
assert foo # 4
640+
else
641+
assert foo
642+
assert foo
643+
end
644+
else
645+
assert foo
646+
end,
647+
assert(foo)
648+
]
649+
end
650+
end
651+
RUBY
652+
end
653+
453654
def test_does_not_register_offense_when_complex_structure_with_single_assertion
454655
assert_no_offenses(<<~RUBY)
455656
class FooTest < ActiveSupport::TestCase

0 commit comments

Comments
 (0)