From e446e022cf2af9f5ddbd62311b698df896feda46 Mon Sep 17 00:00:00 2001 From: Keegan Roth Date: Tue, 12 Jan 2021 14:06:17 -0500 Subject: [PATCH] Be more permissive when detecting frozen objects Rather than rely on an object's #frozen? method, always attempt to proxy a method when requested. Catch the errors resulting from failed attempts to proxy and reraise (or log) them with helpful messages. One notable use case for this is rails' activerecord's Model objects becoming frozen after be deleted. In reality, only the underlying hash of attributes is frozen and it should still be possible to write proxies for these objects. --- lib/rspec/mocks/method_double.rb | 18 ++++++- lib/rspec/mocks/proxy.rb | 2 +- spec/rspec/mocks/matchers/receive_spec.rb | 59 +++++++++++++++++++---- 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/lib/rspec/mocks/method_double.rb b/lib/rspec/mocks/method_double.rb index 0598a35d4..b6ea518cc 100644 --- a/lib/rspec/mocks/method_double.rb +++ b/lib/rspec/mocks/method_double.rb @@ -2,6 +2,9 @@ module RSpec module Mocks # @private class MethodDouble + # @private TODO: drop in favor of FrozenError in ruby 2.5+ + FROZEN_ERROR_MSG = /can't modify frozen/ + # @private attr_reader :method_name, :object, :expectations, :stubs, :method_stasher @@ -68,6 +71,14 @@ def define_proxy_method end @method_is_proxied = true + rescue RuntimeError, TypeError => e + # TODO: drop in favor of FrozenError in ruby 2.5+ + # RuntimeError (and FrozenError) for ruby 2.x + # TypeError for ruby 1.x + if FROZEN_ERROR_MSG === e.message + raise ArgumentError, "Cannot proxy frozen objects, rspec-mocks relies on proxies for method stubbing and expectations." + end + raise end # The implementation of the proxied method. Subclasses may override this @@ -80,7 +91,6 @@ def proxy_method_invoked(_obj, *args, &block) # @private def restore_original_method - return show_frozen_warning if object_singleton_class.frozen? return unless @method_is_proxied remove_method_from_definition_target @@ -88,6 +98,12 @@ def restore_original_method restore_original_visibility @method_is_proxied = false + rescue RuntimeError, TypeError => e + # TODO: drop in favor of FrozenError in ruby 2.5+ + # RuntimeError (and FrozenError) for ruby 2.x + # TypeError for ruby 1.x + return show_frozen_warning if FROZEN_ERROR_MSG === e.message + raise end # @private diff --git a/lib/rspec/mocks/proxy.rb b/lib/rspec/mocks/proxy.rb index d8f092d8c..01e8ad315 100644 --- a/lib/rspec/mocks/proxy.rb +++ b/lib/rspec/mocks/proxy.rb @@ -35,7 +35,7 @@ def initialize(object, order_group, options={}) # @private def ensure_can_be_proxied!(object) - return unless object.is_a?(Symbol) || object.frozen? + return unless object.is_a?(Symbol) || (object.is_a?(String) && object.frozen?) return if object.nil? msg = "Cannot proxy frozen objects" diff --git a/spec/rspec/mocks/matchers/receive_spec.rb b/spec/rspec/mocks/matchers/receive_spec.rb index d91404dec..9edf0a7f9 100644 --- a/spec/rspec/mocks/matchers/receive_spec.rb +++ b/spec/rspec/mocks/matchers/receive_spec.rb @@ -285,8 +285,40 @@ def receiver.method_missing(*); end # a poor man's stub... target.to receive(:foo).and_return(:baz) expect { reset object }.to change { object.foo }.from(:baz).to(:bar) end + end + + shared_examples "resets partial mocks of any instance cleanly" do + let(:klass) { Struct.new(:foo) } + let(:object) { klass.new :bar } - context "on a frozen object" do + it "removes the method double" do + target.to receive(:foo).and_return(:baz) + expect { + verify_all + }.to change { object.foo }.from(:baz).to(:bar) + end + end + + shared_examples "handles frozen objects cleanly" do + let(:klass) { Struct.new(:foo) } + let(:object) { klass.new :bar } + + before do + if RUBY_VERSION < "2.2" && RUBY_VERSION >= "2.0" && RSpec::Support::Ruby.mri? + pending "Does not work on 2.0-2.1 because frozen structs can be modified" + end + end + + context "when adding the method double" do + it "raises clear error" do + object.freeze + expect { + target.to receive(:foo).and_return(:baz) + }.to raise_error(ArgumentError, /Cannot proxy frozen objects/) + end + end + + context "when removing the method double" do it "warns about being unable to remove the method double" do target.to receive(:foo).and_return(:baz) expect_warning_without_call_site(/rspec-mocks was unable to restore the original `foo` method on #{object.inspect}/) @@ -302,20 +334,21 @@ def receiver.method_missing(*); end # a poor man's stub... reset object end end - end - shared_examples "resets partial mocks of any instance cleanly" do - let(:klass) { Struct.new(:foo) } - let(:object) { klass.new :bar } + context "with fake frozen object" do + let(:klass) { Struct.new(:foo, :frozen?, :freeze) } + let(:object) { klass.new :bar, true } - it "removes the method double" do - target.to receive(:foo).and_return(:baz) - expect { - verify_all - }.to change { object.foo }.from(:baz).to(:bar) + it "allows the caller to configure how the subject responds" do + object.freeze + target.to receive(:foo).and_return(5) + expect(object.foo).to eq(5) + expect { reset object }.to change { object.foo }.from(5).to(:bar) + end end end + describe "allow(...).to receive" do it_behaves_like "an expect syntax allowance" do let(:receiver) { double } @@ -324,6 +357,9 @@ def receiver.method_missing(*); end # a poor man's stub... it_behaves_like "resets partial mocks cleanly" do let(:target) { allow(object) } end + it_behaves_like "handles frozen objects cleanly" do + let(:target) { allow(object) } + end context 'ordered with receive counts' do specify 'is not supported' do @@ -426,6 +462,9 @@ def receiver.method_missing(*); end # a poor man's stub... it_behaves_like "resets partial mocks cleanly" do let(:target) { expect(object) } end + it_behaves_like "handles frozen objects cleanly" do + let(:target) { expect(object) } + end context "ordered with receive counts" do let(:dbl) { double(:one => 1, :two => 2) }