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

Mock#expect unexpected behavior with Hash argument #928

Open
speckins opened this issue Oct 9, 2022 · 1 comment
Open

Mock#expect unexpected behavior with Hash argument #928

speckins opened this issue Oct 9, 2022 · 1 comment
Assignees

Comments

@speckins
Copy link

speckins commented Oct 9, 2022

I started getting some "Using the last argument as keyword parameters is deprecated" warnings from a test I was writing with Minitest::Mock. The method being mocked doesn't expect any keyword arguments, just a single Hash.

Here are a few tests demonstrating the code that elicits those warnings as well as a few other things I wouldn't expect (e.g., passing a copy of the hash). These pass in 5.15.0 with no warnings but fail in 5.16.x. I'm using Ruby 2.7.4.

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  #gem 'minitest', '=5.15.0'
  gem 'minitest', '=5.16.3'
end

require 'minitest/autorun'
require 'minitest/mock'

class MockExpectHashTest < Minitest::Test
  def test_hash_argument_with_block
    expected_options = {one: 1}
    mock = Minitest::Mock.new
    mock.expect(:options, expected_options) do |options|
      options == expected_options
    end
    # This line causes a warning with minitest >=5.16.0:
    #   warning: Using the last argument as keyword parameters is
    #   deprecated; maybe ** should be added to the call
    mock.options(expected_options)
    mock.verify
  end

  def test_hash_argument
    expected_options = {one: 1}
    mock = Minitest::Mock.new
    mock.expect(:options, expected_options, [expected_options])
    # This line causes a warning with minitest >=5.16.0:
    #   warning: Using the last argument as keyword parameters is
    #   deprecated; maybe ** should be added to the call
    # It also fails with:
    #   ArgumentError: mocked method :options expects 1 arguments, got []
    mock.options(expected_options)
    mock.verify
  end

  def test_hash_argument_with_block_and_equal?
    expected_options = {one: 1}
    mock = Minitest::Mock.new
    mock.expect(:options, expected_options) do |options|
      options.equal? expected_options
    end
    # Fails w/ minitest >=5.16.0
    mock.options(expected_options)
    mock.verify
  end

  def test_frozen_hash?
    expected_options = {one: 1}.freeze
    mock = Minitest::Mock.new
    mock.expect(:options, expected_options) do |options|
      options.frozen?
    end
    # Fails w/ minitest >=5.16.0
    mock.options(expected_options)
    mock.verify
  end

  # edited to add this test
  def test_hash_with_default_proc
    expected_hash = Hash.new(2)
    mock = Minitest::Mock.new
    mock.expect(:options, expected_hash) do |options|
      options[:foo] == 2
    end
    # Fails w/ minitest >=5.16.0
    mock.options(expected_hash)
    mock.verify
  end
end

It seems that the "kwargs" hack breaks passing hashes normally?

@speckins
Copy link
Author

speckins commented Oct 11, 2022

As far as I can tell, this is caused by #method_missing's **kwargs parameter.

What about something like this, added to the Minitest::Mock class after the definition of method_missing?

if RUBY_VERSION < '3.0'
  alias __method_missing method_missing

  def method_missing sym, *args, &block
    # Let the original method handle this.
    return __method_missing(sym, *args) unless @expected_calls.key?(sym)

    index = @actual_calls[sym].length
    expected_call = @expected_calls[sym][index]

    # Let the original method handle this, too.
    return __method_missing(sym, *args) unless expected_call

    expected_kwargs = expected_call[:kwargs]
    val_block       = expected_call[:block]

    # Is this call expecting keyword arguments?
    expecting_kwargs =
      expected_kwargs &&
      (expected_kwargs == Hash || !expected_kwargs.empty?) ||
      val_block &&
      val_block.parameters.any? { |type, _| type.to_s.start_with? 'key' }

    # If not, or if none are present, use an empty Hash.
    kwargs = expecting_kwargs && Hash === args.last ? args.pop : {}

    __method_missing(sym, *args, **kwargs, &block)
  end
end

I tested this on 4accdd5 with Ruby 2.5.1 and 2.7.4. My tests above pass, and only one of the test/minitest/test_minitest_mock.rb tests fails, test_mock_block_is_passed_keyword_args__args__old_style_bad. (That test is because of the current behavior of assuming that if the last arg is a hash, it is keyword args?)

@zenspider zenspider self-assigned this Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants