From 77b64fe37c440caa008dbc29c25e4e356711f4a5 Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Sun, 30 Jun 2013 22:53:06 -0400 Subject: [PATCH 1/4] Resolves rspec/rspec-core#950 and adds specs * Hopefully the code clarity is improved here * We also change the behavior by *not* duping an object if `space` is not yet initialized. This should have been the behavior from the beginning, but it was overlooked and did not have specs. --- lib/rspec/mocks/extensions/marshal.rb | 14 +++--- spec/rspec/mocks/extensions/marshal_spec.rb | 48 +++++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 spec/rspec/mocks/extensions/marshal_spec.rb diff --git a/lib/rspec/mocks/extensions/marshal.rb b/lib/rspec/mocks/extensions/marshal.rb index 7698c1887..e0cc4d60d 100644 --- a/lib/rspec/mocks/extensions/marshal.rb +++ b/lib/rspec/mocks/extensions/marshal.rb @@ -1,13 +1,13 @@ module Marshal class << self - def dump_with_mocks(*args) - object = args.shift - - if ( ::RSpec::Mocks.space && !::RSpec::Mocks.space.registered?(object) ) || NilClass === object - return dump_without_mocks(*args.unshift(object)) + # Duplicates any mock objects before serialization. Otherwise, + # serialization will fail because methods exist on the singleton class. + def dump_with_mocks(object, *rest) + if !::RSpec::Mocks.space || !::RSpec::Mocks.space.registered?(object) || NilClass === object + dump_without_mocks(object, *rest) + else + dump_without_mocks(object.dup, *rest) end - - dump_without_mocks(*args.unshift(object.dup)) end alias_method :dump_without_mocks, :dump diff --git a/spec/rspec/mocks/extensions/marshal_spec.rb b/spec/rspec/mocks/extensions/marshal_spec.rb new file mode 100644 index 000000000..cce039a3c --- /dev/null +++ b/spec/rspec/mocks/extensions/marshal_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +describe Marshal, 'extensions' do + # An object that raises when code attempts to dup it. + # + # Because we manipulate the internals of RSpec::Mocks.space below, we need + # an object that simply blows up when #dup is called without using any + # partial mocking or stubbing from rspec-mocks itself. + class UndupableObject + def dup + raise NotImplementedError + end + end + + describe '#dump' do + context 'when rspec-mocks has not been fully initialized' do + def without_space + stashed_space, RSpec::Mocks.space = RSpec::Mocks.space, nil + yield + ensure + RSpec::Mocks.space = stashed_space + end + + it 'does not duplicate the object before serialization' do + obj = UndupableObject.new + without_space do + expect { Marshal.dump(obj) }.not_to raise_error + end + end + end + + context 'when rspec-mocks has been fully initialized' do + it 'duplicates objects with stubbed or mocked implementations before serialization' do + obj = double(:foo => "bar") + expect { Marshal.dump(obj) }.not_to raise_error + end + + it 'does not duplicate other objects before serialization' do + obj = UndupableObject.new + expect { Marshal.dump(obj) }.not_to raise_error + end + + it 'does not duplicate nil before serialization' do + expect { Marshal.dump(nil) }.not_to raise_error + end + end + end +end From 82ada28657f206ef317c46c94a5aa39409135a5f Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Mon, 1 Jul 2013 21:09:55 -0400 Subject: [PATCH 2/4] Improves clarity of nil check --- lib/rspec/mocks/extensions/marshal.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rspec/mocks/extensions/marshal.rb b/lib/rspec/mocks/extensions/marshal.rb index e0cc4d60d..b8cf7b5df 100644 --- a/lib/rspec/mocks/extensions/marshal.rb +++ b/lib/rspec/mocks/extensions/marshal.rb @@ -3,7 +3,7 @@ class << self # Duplicates any mock objects before serialization. Otherwise, # serialization will fail because methods exist on the singleton class. def dump_with_mocks(object, *rest) - if !::RSpec::Mocks.space || !::RSpec::Mocks.space.registered?(object) || NilClass === object + if ::RSpec::Mocks.space.nil? || !::RSpec::Mocks.space.registered?(object) || NilClass === object dump_without_mocks(object, *rest) else dump_without_mocks(object.dup, *rest) From ed527b92b9437dab68cdc7bcb3c98ddefb44dd0d Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Mon, 1 Jul 2013 21:16:11 -0400 Subject: [PATCH 3/4] Improves strength of assertions per @myronmarston's suggestions --- spec/rspec/mocks/extensions/marshal_spec.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/spec/rspec/mocks/extensions/marshal_spec.rb b/spec/rspec/mocks/extensions/marshal_spec.rb index cce039a3c..46f436b34 100644 --- a/spec/rspec/mocks/extensions/marshal_spec.rb +++ b/spec/rspec/mocks/extensions/marshal_spec.rb @@ -24,7 +24,8 @@ def without_space it 'does not duplicate the object before serialization' do obj = UndupableObject.new without_space do - expect { Marshal.dump(obj) }.not_to raise_error + serialized = Marshal.dump(obj) + expect(Marshal.load(serialized)).to be_an(UndupableObject) end end end @@ -32,16 +33,21 @@ def without_space context 'when rspec-mocks has been fully initialized' do it 'duplicates objects with stubbed or mocked implementations before serialization' do obj = double(:foo => "bar") - expect { Marshal.dump(obj) }.not_to raise_error + + serialized = Marshal.dump(obj) + expect(Marshal.load(serialized)).to be_an(obj.class) end it 'does not duplicate other objects before serialization' do obj = UndupableObject.new - expect { Marshal.dump(obj) }.not_to raise_error + + serialized = Marshal.dump(obj) + expect(Marshal.load(serialized)).to be_an(UndupableObject) end it 'does not duplicate nil before serialization' do - expect { Marshal.dump(nil) }.not_to raise_error + serialized = Marshal.dump(nil) + expect(Marshal.load(serialized)).to be_nil end end end From bcee05639bc4668959485dc8627132d29e96e3e8 Mon Sep 17 00:00:00 2001 From: Andy Lindeman Date: Mon, 1 Jul 2013 23:01:39 -0400 Subject: [PATCH 4/4] Changelog entry --- Changelog.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Changelog.md b/Changelog.md index 74b73e86c..a24d4dc83 100644 --- a/Changelog.md +++ b/Changelog.md @@ -18,6 +18,9 @@ Bug Fixes: `and_yield`, `and_raise`, `and_return` or `and_throw`. This got fixed in 2.13.1 but failed to get merged into master for the 2.14.0.rc1 release (Myron Marston). +* `Marshal.dump` does not unnecessarily duplicate objects when rspec-mocks has + not been fully initialized. This could cause errors when using `spork` or + similar preloading gems (Andy Lindeman). ### 2.14.0.rc1 / 2013-05-27 [full changelog](http://github.com/rspec/rspec-mocks/compare/v2.13.0...v2.14.0.rc1)