`assert_called_with` should require `args` argument
There are two main reasons why `assert_called_with` should require
`args` argument:
1) If we want to assert that some method should be called and we don't
need to check with which arguments it should be called then we should use
`assert_called`.
2) `assert_called_with` without `args` argument doesn't assert anything!
```ruby
assert_called_with(@object, :increment) do
@object.decrement
end
```
It causes false assertions in tests that could cause regressions in the project.
I found this bug by working on
[minitest-mock_expectations](https://github.com/bogdanvlviv/minitest-mock_expectations) gem.
This gem is an extension for minitest that provides almost the same method call
assertions.
I was wondering whether you would consider adding "minitest-mock_expectations"
to `rails/rails` instead of private `ActiveSupport::Testing::MethodCallAssertions` module.
If yes, I'll send a patch - a970ecc42c
This commit is contained in:
parent
a3dcba42e2
commit
9d0cf52096
|
@ -272,7 +272,7 @@ if current_adapter?(:Mysql2Adapter)
|
|||
|
||||
def test_db_retrieves_collation
|
||||
ActiveRecord::Base.stub(:connection, @connection) do
|
||||
assert_called_with(@connection, :collation) do
|
||||
assert_called(@connection, :collation) do
|
||||
ActiveRecord::Tasks::DatabaseTasks.collation @configuration
|
||||
end
|
||||
end
|
||||
|
|
|
@ -17,7 +17,7 @@ module ActiveSupport
|
|||
assert_equal times, times_called, error
|
||||
end
|
||||
|
||||
def assert_called_with(object, method_name, args = [], returns: nil)
|
||||
def assert_called_with(object, method_name, args, returns: nil)
|
||||
mock = Minitest::Mock.new
|
||||
|
||||
if args.all? { |arg| arg.is_a?(Array) }
|
||||
|
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
module CacheInstrumentationBehavior
|
||||
def test_fetch_multi_uses_write_multi_entries_store_provider_interface
|
||||
assert_called_with(@cache, :write_multi_entries) do
|
||||
assert_called(@cache, :write_multi_entries) do
|
||||
@cache.fetch_multi "a", "b", "c" do |key|
|
||||
key * 2
|
||||
end
|
||||
|
|
|
@ -60,12 +60,6 @@ class MethodCallAssertionsTest < ActiveSupport::TestCase
|
|||
assert_match(/dang it.\nExpected increment/, error.message)
|
||||
end
|
||||
|
||||
def test_assert_called_with
|
||||
assert_called_with(@object, :increment) do
|
||||
@object.increment
|
||||
end
|
||||
end
|
||||
|
||||
def test_assert_called_with_arguments
|
||||
assert_called_with(@object, :<<, [ 2 ]) do
|
||||
@object << 2
|
||||
|
@ -88,12 +82,6 @@ class MethodCallAssertionsTest < ActiveSupport::TestCase
|
|||
end
|
||||
end
|
||||
|
||||
def test_assert_called_with_returns
|
||||
assert_called_with(@object, :increment, returns: 1) do
|
||||
@object.increment
|
||||
end
|
||||
end
|
||||
|
||||
def test_assert_called_with_multiple_expected_arguments
|
||||
assert_called_with(@object, :<<, [ [ 1 ], [ 2 ] ]) do
|
||||
@object << 1
|
||||
|
|
Loading…
Reference in New Issue