Permalink
Browse files

Add method_call_assertions and use them instead of Mocha

Six Mocha calls prove quite resistant to Minitestification. For example,
if we replace

```
  ActiveRecord::Associations::HasManyAssociation
    .any_instance
    .expects(:reader)
    .never
```

with `assert_not_called`, Minitest wisely raises

```
NameError: undefined method `reader' for class `ActiveRecord::Associations::HasManyAssociation'
```

as `:reader` comes from a deeply embedded abstract class,
`ActiveRecord::Associations::CollectionAssociation`.

This patch tackles this difficulty by adding
`ActiveSupport::Testing::MethodCallAsserts#assert_called_on_instance_of`
which injects a stubbed method into `klass`, and verifies the number of
times it is called, similar to `assert_called`. It also adds  a convenience
method, `assert_not_called_on_instance_of`, mirroring
`assert_not_called`.

It uses the new method_call_assertions to replace the remaining Mocha
calls in `ActiveRecord` tests.

[utilum + bogdanvlviv + kspath]
  • Loading branch information...
utilum committed Jul 23, 2018
1 parent bef6c8b commit a72bca82301bc4851f40945f85711f5cefd10178
@@ -1571,8 +1571,9 @@ def test_preloading_has_many_through_with_custom_scope
# CollectionProxy#reader is expensive, so the preloader avoids calling it.
test "preloading has_many_through association avoids calling association.reader" do
ActiveRecord::Associations::HasManyAssociation.any_instance.expects(:reader).never
Author.preload(:readonly_comments).first!
assert_not_called_on_instance_of(ActiveRecord::Associations::HasManyAssociation, :reader) do
Author.preload(:readonly_comments).first!
end
end
test "preloading through a polymorphic association doesn't require the association to exist" do
@@ -2134,21 +2134,29 @@ def test_creating_using_primary_key
end
def test_defining_has_many_association_with_delete_all_dependency_lazily_evaluates_target_class
ActiveRecord::Reflection::AssociationReflection.any_instance.expects(:class_name).never
class_eval(<<-EOF, __FILE__, __LINE__ + 1)
class DeleteAllModel < ActiveRecord::Base
has_many :nonentities, :dependent => :delete_all
end
EOF
assert_not_called_on_instance_of(
ActiveRecord::Reflection::AssociationReflection,
:class_name,
) do
class_eval(<<-EOF, __FILE__, __LINE__ + 1)
class DeleteAllModel < ActiveRecord::Base
has_many :nonentities, :dependent => :delete_all
end
EOF
end
end
def test_defining_has_many_association_with_nullify_dependency_lazily_evaluates_target_class
ActiveRecord::Reflection::AssociationReflection.any_instance.expects(:class_name).never
class_eval(<<-EOF, __FILE__, __LINE__ + 1)
class NullifyModel < ActiveRecord::Base
has_many :nonentities, :dependent => :nullify
end
EOF
assert_not_called_on_instance_of(
ActiveRecord::Reflection::AssociationReflection,
:class_name,
) do
class_eval(<<-EOF, __FILE__, __LINE__ + 1)
class NullifyModel < ActiveRecord::Base
has_many :nonentities, :dependent => :nullify
end
EOF
end
end
def test_attributes_are_being_set_when_initialized_from_has_many_association_with_where_clause
@@ -183,5 +183,3 @@ def in_time_zone(zone)
ActiveRecord::Base.time_zone_aware_attributes = old_tz
end
end
require "mocha/minitest" # FIXME: stop using mocha
@@ -46,46 +46,61 @@ def with_stubbed_new
class DatabaseTasksUtilsTask < ActiveRecord::TestCase
def test_raises_an_error_when_called_with_protected_environment
ActiveRecord::MigrationContext.any_instance.stubs(:current_version).returns(1)
protected_environments = ActiveRecord::Base.protected_environments
current_env = ActiveRecord::Base.connection.migration_context.current_environment
assert_not_includes protected_environments, current_env
# Assert no error
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
ActiveRecord::Base.protected_environments = [current_env]
assert_raise(ActiveRecord::ProtectedEnvironmentError) do
assert_called_on_instance_of(
ActiveRecord::MigrationContext,
:current_version,
times: 6,
returns: 1
) do
assert_not_includes protected_environments, current_env
# Assert no error
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
ActiveRecord::Base.protected_environments = [current_env]
assert_raise(ActiveRecord::ProtectedEnvironmentError) do
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
end
end
ensure
ActiveRecord::Base.protected_environments = protected_environments
end
def test_raises_an_error_when_called_with_protected_environment_which_name_is_a_symbol
ActiveRecord::MigrationContext.any_instance.stubs(:current_version).returns(1)
protected_environments = ActiveRecord::Base.protected_environments
current_env = ActiveRecord::Base.connection.migration_context.current_environment
assert_not_includes protected_environments, current_env
# Assert no error
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
ActiveRecord::Base.protected_environments = [current_env.to_sym]
assert_raise(ActiveRecord::ProtectedEnvironmentError) do
assert_called_on_instance_of(
ActiveRecord::MigrationContext,
:current_version,
times: 6,
returns: 1
) do
assert_not_includes protected_environments, current_env
# Assert no error
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
ActiveRecord::Base.protected_environments = [current_env.to_sym]
assert_raise(ActiveRecord::ProtectedEnvironmentError) do
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
end
end
ensure
ActiveRecord::Base.protected_environments = protected_environments
end
def test_raises_an_error_if_no_migrations_have_been_made
ActiveRecord::InternalMetadata.stub(:table_exists?, false) do
ActiveRecord::MigrationContext.any_instance.stubs(:current_version).returns(1)
assert_raise(ActiveRecord::NoEnvironmentInSchemaError) do
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
assert_called_on_instance_of(
ActiveRecord::MigrationContext,
:current_version,
returns: 1
) do
assert_raise(ActiveRecord::NoEnvironmentInSchemaError) do
ActiveRecord::Tasks::DatabaseTasks.check_protected_environments!
end
end
end
end
@@ -35,6 +35,35 @@ def assert_not_called(object, method_name, message = nil, &block)
assert_called(object, method_name, message, times: 0, &block)
end
# TODO: No need to resort to #send once support for Ruby 2.4 is
# dropped.
def assert_called_on_instance_of(klass, method_name, message = nil, times: 1, returns: nil)
times_called = 0
klass.send(:define_method, "stubbed_#{method_name}") do |*|
times_called += 1
returns
end
klass.send(:alias_method, "original_#{method_name}", method_name)
klass.send(:alias_method, method_name, "stubbed_#{method_name}")
yield
error = "Expected #{method_name} to be called #{times} times, but was called #{times_called} times"
error = "#{message}.\n#{error}" if message
assert_equal times, times_called, error
ensure
klass.send(:alias_method, method_name, "original_#{method_name}")
klass.send(:undef_method, "original_#{method_name}")
klass.send(:undef_method, "stubbed_#{method_name}")
end
def assert_not_called_on_instance_of(klass, method_name, message = nil, &block)
assert_called_on_instance_of(klass, method_name, message, times: 0, &block)
end
def stub_any_instance(klass, instance: klass.new)
klass.stub(:new, instance) { yield instance }
end
@@ -91,6 +91,65 @@ def test_assert_called_with_multiple_expected_arguments
end
end
def test_assert_called_on_instance_of_with_defaults_to_expect_once
assert_called_on_instance_of Level, :increment do
@object.increment
end
end
def test_assert_called_on_instance_of_more_than_once
assert_called_on_instance_of(Level, :increment, times: 2) do
@object.increment
@object.increment
end
end
def test_assert_called_on_instance_of_with_arguments
assert_called_on_instance_of(Level, :<<) do
@object << 2
end
end
def test_assert_called_on_instance_of_returns
assert_called_on_instance_of(Level, :increment, returns: 10) do
assert_equal 10, @object.increment
end
assert_equal 1, @object.increment
end
def test_assert_called_on_instance_of_failure
error = assert_raises(Minitest::Assertion) do
assert_called_on_instance_of(Level, :increment) do
# Call nothing...
end
end
assert_equal "Expected increment to be called 1 times, but was called 0 times.\nExpected: 1\n Actual: 0", error.message
end
def test_assert_called_on_instance_of_with_message
error = assert_raises(Minitest::Assertion) do
assert_called_on_instance_of(Level, :increment, "dang it") do
# Call nothing...
end
end
assert_match(/dang it.\nExpected increment/, error.message)
end
def test_assert_called_on_instance_of_nesting
assert_called_on_instance_of(Level, :increment, times: 3) do
assert_called_on_instance_of(Level, :decrement, times: 2) do
@object.increment
@object.decrement
@object.increment
@object.decrement
@object.increment
end
end
end
def test_assert_not_called
assert_not_called(@object, :decrement) do
@object.increment
@@ -107,6 +166,30 @@ def test_assert_not_called_failure
assert_equal "Expected increment to be called 0 times, but was called 1 times.\nExpected: 0\n Actual: 1", error.message
end
def test_assert_not_called_on_instance_of
assert_not_called_on_instance_of(Level, :decrement) do
@object.increment
end
end
def test_assert_not_called_on_instance_of_failure
error = assert_raises(Minitest::Assertion) do
assert_not_called_on_instance_of(Level, :increment) do
@object.increment
end
end
assert_equal "Expected increment to be called 0 times, but was called 1 times.\nExpected: 0\n Actual: 1", error.message
end
def test_assert_not_called_on_instance_of_nesting
assert_not_called_on_instance_of(Level, :increment) do
assert_not_called_on_instance_of(Level, :decrement) do
# Call nothing...
end
end
end
def test_stub_any_instance
stub_any_instance(Level) do |instance|
assert_equal instance, Level.new

0 comments on commit a72bca8

Please sign in to comment.