bogdanvlviv/minitest-mock_expectations

Asserting call to method with array argument fails

thatguysimon opened this issue ยท 3 comments

Hi, great job on this gem!
I found a pretty serious bug though :(

If you try to assert a call to a method that takes an array as its argument:

assert_called_with(Tempfile, :new, [['hello', '.jpg']]) do
  file = Tempfile.new(['hello', '.jpg'])  
end

Running the above code will result in:

ArgumentError: mocked method :call expects 2 arguments, got 1
from .../ruby-2.6.3/gems/minitest-5.14.0/lib/minitest/mock.rb:150:in `method_missing'

This happens due to a collision with the feature of being able to pass an array of different sets of arguments that the method can be called with.

I suggest changing the interface of the multiple arguments feature to the following (which will also allow providing a different return value for each set of arguments):

expectations = [
  { 
    expected_args: ["Thanks for sharing this."],
    return_value: mocked_comment_1
  },
  { 
    expected_args:  ["Thanks!"],
    return_value: mocked_comment_2 
  }
]

assert_called_with(@post, :add_comment, expectations: expectations) do
  @post.add_comment("Thanks for sharing this.")
  @post.add_comment("Thanks!")
end

Shall I open a PR for this?

Hey @thatguysimon, thanks for raising the issue. ๐Ÿ™‡โ€โ™‚๏ธ I'll try to get back to this later this week.

@thatguysimon I opened a PR to fix an issue with the example: #5

assert_called_with(Tempfile, :new, [['hello', '.jpg']]) do
  file = Tempfile.new(['hello', '.jpg'])  
end

Also added tests and docs to clarify assert_called_with. Please let me know what you think about that PR. Thanks.

Hi again @thatguysimon. I tried the same fix on Rails(rails/rails#39033) and we we found the real bug it causes. For instance(rails/rails#39033 (comment)), it isnt' possible to assert:

@object.push([ 1 ], [ 2 ])

anymore.

So to solve you issue with

assert_called_with(Tempfile, :new, [['hello', '.jpg']]) do
  file = Tempfile.new(['hello', '.jpg'])  
end

you should change your code to

assert_called_with(Tempfile, :new, [ [['hello', '.jpg']] ]) do
  file = Tempfile.new(['hello', '.jpg'])  
end

I gonna revert this change and release 1.1.3 ๐Ÿ˜… because of that:

#7
https://github.com/bogdanvlviv/minitest-mock_expectations/releases/tag/v1.1.3
https://rubygems.org/gems/minitest-mock_expectations/versions/1.1.3