From 5b35444c5af80d287048810f1240f32e8bb91a36 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Thu, 12 Nov 2020 22:48:54 +0300 Subject: [PATCH 01/26] Remove usages of module_prepends_supported? --- lib/rspec/mocks/any_instance/recorder.rb | 16 ++-- lib/rspec/mocks/method_double.rb | 59 ++++++--------- lib/rspec/mocks/proxy.rb | 20 +++-- spec/rspec/mocks/any_instance_spec.rb | 4 +- spec/rspec/mocks/stub_spec.rb | 96 ++++++++++++------------ 5 files changed, 86 insertions(+), 109 deletions(-) diff --git a/lib/rspec/mocks/any_instance/recorder.rb b/lib/rspec/mocks/any_instance/recorder.rb index d30cc86cb..c5b397d67 100644 --- a/lib/rspec/mocks/any_instance/recorder.rb +++ b/lib/rspec/mocks/any_instance/recorder.rb @@ -275,18 +275,12 @@ def mark_invoked!(method_name) end end - if Support::RubyFeatures.module_prepends_supported? - def allow_no_prepended_module_definition_of(method_name) - prepended_modules = RSpec::Mocks::Proxy.prepended_modules_of(@klass) - problem_mod = prepended_modules.find { |mod| mod.method_defined?(method_name) } - return unless problem_mod + def allow_no_prepended_module_definition_of(method_name) + prepended_modules = RSpec::Mocks::Proxy.prepended_modules_of(@klass) + problem_mod = prepended_modules.find { |mod| mod.method_defined?(method_name) } + return unless problem_mod - AnyInstance.error_generator.raise_not_supported_with_prepend_error(method_name, problem_mod) - end - else - def allow_no_prepended_module_definition_of(_method_name) - # nothing to do; prepends aren't supported on this version of ruby - end + AnyInstance.error_generator.raise_not_supported_with_prepend_error(method_name, problem_mod) end end end diff --git a/lib/rspec/mocks/method_double.rb b/lib/rspec/mocks/method_double.rb index 4ffdcdf26..8d1f892af 100644 --- a/lib/rspec/mocks/method_double.rb +++ b/lib/rspec/mocks/method_double.rb @@ -205,6 +205,8 @@ def raise_method_not_stubbed_error RSpec::Mocks.error_generator.raise_method_not_stubbed_error(method_name) end + private + # In Ruby 2.0.0 and above prepend will alter the method lookup chain. # We use an object's singleton class to define method doubles upon, # however if the object has had its singleton class (as opposed to @@ -216,51 +218,36 @@ def raise_method_not_stubbed_error # that is either the singleton class, or if necessary, a prepended module # of our own. # - if Support::RubyFeatures.module_prepends_supported? - - private - # We subclass `Module` in order to be able to easily detect our prepended module. - RSpecPrependedModule = Class.new(Module) - - def definition_target - @definition_target ||= usable_rspec_prepended_module || object_singleton_class - end + # We subclass `Module` in order to be able to easily detect our prepended module. + RSpecPrependedModule = Class.new(Module) - def usable_rspec_prepended_module - @proxy.prepended_modules_of_singleton_class.each do |mod| - # If we have one of our modules prepended before one of the user's - # modules that defines the method, use that, since our module's - # definition will take precedence. - return mod if RSpecPrependedModule === mod - - # If we hit a user module with the method defined first, - # we must create a new prepend module, even if one exists later, - # because ours will only take precedence if it comes first. - return new_rspec_prepended_module if mod.method_defined?(method_name) - end - - nil - end + def definition_target + @definition_target ||= usable_rspec_prepended_module || object_singleton_class + end - def new_rspec_prepended_module - RSpecPrependedModule.new.tap do |mod| - object_singleton_class.__send__ :prepend, mod - end + def usable_rspec_prepended_module + @proxy.prepended_modules_of_singleton_class.each do |mod| + # If we have one of our modules prepended before one of the user's + # modules that defines the method, use that, since our module's + # definition will take precedence. + return mod if RSpecPrependedModule === mod + + # If we hit a user module with the method defined first, + # we must create a new prepend module, even if one exists later, + # because ours will only take precedence if it comes first. + return new_rspec_prepended_module if mod.method_defined?(method_name) end - else - - private + nil + end - def definition_target - object_singleton_class + def new_rspec_prepended_module + RSpecPrependedModule.new.tap do |mod| + object_singleton_class.__send__ :prepend, mod end - end - private - def remove_method_from_definition_target definition_target.__send__(:remove_method, @method_name) rescue NameError diff --git a/lib/rspec/mocks/proxy.rb b/lib/rspec/mocks/proxy.rb index 6d3583c5b..699ef6a3e 100644 --- a/lib/rspec/mocks/proxy.rb +++ b/lib/rspec/mocks/proxy.rb @@ -231,20 +231,18 @@ def visibility_for(_method_name) :public end - if Support::RubyFeatures.module_prepends_supported? - def self.prepended_modules_of(klass) - ancestors = klass.ancestors + def self.prepended_modules_of(klass) + ancestors = klass.ancestors - # `|| 0` is necessary for Ruby 2.0, where the singleton class - # is only in the ancestor list when there are prepended modules. - singleton_index = ancestors.index(klass) || 0 + # `|| 0` is necessary for Ruby 2.0, where the singleton class + # is only in the ancestor list when there are prepended modules. + singleton_index = ancestors.index(klass) || 0 - ancestors[0, singleton_index] - end + ancestors[0, singleton_index] + end - def prepended_modules_of_singleton_class - @prepended_modules_of_singleton_class ||= RSpec::Mocks::Proxy.prepended_modules_of(@object.singleton_class) - end + def prepended_modules_of_singleton_class + @prepended_modules_of_singleton_class ||= RSpec::Mocks::Proxy.prepended_modules_of(@object.singleton_class) end # @private diff --git a/spec/rspec/mocks/any_instance_spec.rb b/spec/rspec/mocks/any_instance_spec.rb index 656f7d904..1f63bf4ce 100644 --- a/spec/rspec/mocks/any_instance_spec.rb +++ b/spec/rspec/mocks/any_instance_spec.rb @@ -168,7 +168,7 @@ def private_method; :private_method_return_value; end end end - context "when the class has a prepended module", :if => Support::RubyFeatures.module_prepends_supported? do + context "when the class has a prepended module" do it 'allows stubbing a method that is not defined on the prepended module' do klass.class_eval { prepend Module.new { def other; end } } allow_any_instance_of(klass).to receive(:foo).and_return(45) @@ -582,7 +582,7 @@ def inspect expect(object.foo).to eq(3) end - context "when the class has a prepended module", :if => Support::RubyFeatures.module_prepends_supported? do + context "when the class has a prepended module" do it 'allows mocking a method that is not defined on the prepended module' do klass.class_eval { prepend Module.new { def other; end } } expect_any_instance_of(klass).to receive(:foo).and_return(45) diff --git a/spec/rspec/mocks/stub_spec.rb b/spec/rspec/mocks/stub_spec.rb index efa82d37a..c84978d3f 100644 --- a/spec/rspec/mocks/stub_spec.rb +++ b/spec/rspec/mocks/stub_spec.rb @@ -133,7 +133,7 @@ def method_missing(name, *) end end - context "stubbing with prepend", :if => Support::RubyFeatures.module_prepends_supported? do + context "stubbing with prepend" do module ToBePrepended def value "#{super}_prepended".to_sym @@ -374,71 +374,69 @@ def self.method_b expect(b.method_b).to eql("stubbed method_b") end - if Support::RubyFeatures.module_prepends_supported? - context "with a prepended module (ruby 2.0.0+)" do - module ToBePrepended - def existing_method - "#{super}_prepended".to_sym - end + context "with a prepended module" do + module ToBePrepended + def existing_method + "#{super}_prepended".to_sym end + end - before do - @prepended_class = Class.new do - prepend ToBePrepended + before do + @prepended_class = Class.new do + prepend ToBePrepended - def existing_method - :original_value - end + def existing_method + :original_value + end - def non_prepended_method - :not_prepended - end + def non_prepended_method + :not_prepended end - @prepended_instance = @prepended_class.new end + @prepended_instance = @prepended_class.new + end - it "restores prepended instance methods" do - allow(@prepended_instance).to receive(:existing_method) { :stubbed } - expect(@prepended_instance.existing_method).to eq :stubbed + it "restores prepended instance methods" do + allow(@prepended_instance).to receive(:existing_method) { :stubbed } + expect(@prepended_instance.existing_method).to eq :stubbed - reset @prepended_instance - expect(@prepended_instance.existing_method).to eq :original_value_prepended - end + reset @prepended_instance + expect(@prepended_instance.existing_method).to eq :original_value_prepended + end - it "restores non-prepended instance methods" do - allow(@prepended_instance).to receive(:non_prepended_method) { :stubbed } - expect(@prepended_instance.non_prepended_method).to eq :stubbed + it "restores non-prepended instance methods" do + allow(@prepended_instance).to receive(:non_prepended_method) { :stubbed } + expect(@prepended_instance.non_prepended_method).to eq :stubbed - reset @prepended_instance - expect(@prepended_instance.non_prepended_method).to eq :not_prepended - end + reset @prepended_instance + expect(@prepended_instance.non_prepended_method).to eq :not_prepended + end - it "restores prepended class methods" do - klass = Class.new do - class << self; prepend ToBePrepended; end - def self.existing_method - :original_value - end + it "restores prepended class methods" do + klass = Class.new do + class << self; prepend ToBePrepended; end + def self.existing_method + :original_value end + end - allow(klass).to receive(:existing_method) { :stubbed } - expect(klass.existing_method).to eq :stubbed + allow(klass).to receive(:existing_method) { :stubbed } + expect(klass.existing_method).to eq :stubbed - reset klass - expect(klass.existing_method).to eq :original_value_prepended - end + reset klass + expect(klass.existing_method).to eq :original_value_prepended + end - it "restores prepended object singleton methods" do - object = Object.new - def object.existing_method; :original_value; end - object.singleton_class.send(:prepend, ToBePrepended) + it "restores prepended object singleton methods" do + object = Object.new + def object.existing_method; :original_value; end + object.singleton_class.send(:prepend, ToBePrepended) - allow(object).to receive(:existing_method) { :stubbed } - expect(object.existing_method).to eq :stubbed + allow(object).to receive(:existing_method) { :stubbed } + expect(object.existing_method).to eq :stubbed - reset object - expect(object.existing_method).to eq :original_value_prepended - end + reset object + expect(object.existing_method).to eq :original_value_prepended end end end From a5f22d93315bf57790b72c5705583d8d5169e3f9 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 14:21:58 +0300 Subject: [PATCH 02/26] Update required Ruby version gemspec constraint --- rspec-mocks.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rspec-mocks.gemspec b/rspec-mocks.gemspec index 9fd5e9a95..d81ad29e4 100644 --- a/rspec-mocks.gemspec +++ b/rspec-mocks.gemspec @@ -27,7 +27,7 @@ Gem::Specification.new do |s| s.rdoc_options = ["--charset=UTF-8"] s.require_path = "lib" - s.required_ruby_version = '>= 1.8.7' + s.required_ruby_version = '>= 2.3.0' private_key = File.expand_path('~/.gem/rspec-gem-private_key.pem') if File.exist?(private_key) From 74ff06d2a209076c331069bb0a6c339abe80bb1e Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 14:27:52 +0300 Subject: [PATCH 03/26] Remove explicit gem version constraints --- Gemfile | 42 +++--------------------------------------- rspec-mocks.gemspec | 2 +- 2 files changed, 4 insertions(+), 40 deletions(-) diff --git a/Gemfile b/Gemfile index b8c1ac2eb..b9144bab0 100644 --- a/Gemfile +++ b/Gemfile @@ -12,20 +12,14 @@ branch = File.read(File.expand_path("../maintenance-branch", __FILE__)).chomp end end -if RUBY_VERSION < '1.9.3' - gem 'rake', '< 11.0.0' # rake 11 requires Ruby 1.9.3 or later -elsif RUBY_VERSION < '2.0.0' - gem 'rake', '< 12.0.0' # rake 12 requires Ruby 2.0.0 or later -else - gem 'rake', '> 12.3.2' -end - if ENV['DIFF_LCS_VERSION'] gem 'diff-lcs', ENV['DIFF_LCS_VERSION'] else gem 'diff-lcs', '~> 1.4', '>= 1.4.3' end +gem 'ffi', '> 1.9.24' # prevent Github security vulnerability warning + gem 'yard', '~> 0.9.24', :require => false # No need to run rubocop on earlier versions @@ -33,25 +27,10 @@ if RUBY_VERSION >= '2.4' && RUBY_ENGINE == 'ruby' gem 'rubocop', "~> 0.52.1" end -# allow gems to be installed on older rubies and/or windows -if RUBY_VERSION < '2.2.0' && !!(RbConfig::CONFIG['host_os'] =~ /cygwin|mswin|mingw|bccwin|wince|emx/) - gem 'ffi', '< 1.10' -elsif RUBY_VERSION < '1.9' - gem 'ffi', '< 1.9.19' # ffi dropped Ruby 1.8 support in 1.9.19 -elsif RUBY_VERSION < '2.0' - gem 'ffi', '< 1.11.0' # ffi dropped Ruby 1.9 support in 1.11.0 -else - gem 'ffi', '> 1.9.24' # prevent Github security vulnerability warning -end - if RUBY_VERSION <= '2.3.0' && !!(RbConfig::CONFIG['host_os'] =~ /cygwin|mswin|mingw|bccwin|wince|emx/) gem "childprocess", "< 1.0.0" end -if RUBY_VERSION < '1.9.2' - gem 'contracts', '~> 0.15.0' # is a dependency of aruba -end - # Version 5.12 of minitest requires Ruby 2.4 if RUBY_VERSION < '2.4.0' gem 'minitest', '< 5.12.0' @@ -65,21 +44,6 @@ end gem 'simplecov', '~> 0.8' -if RUBY_VERSION < '2.0.0' || RUBY_ENGINE == 'java' - gem 'json', '< 2.0.0' # this is a dependency of simplecov -else - gem 'json', '> 2.3.0' -end - -platforms :jruby do - if RUBY_VERSION < '1.9.0' - # Pin jruby-openssl on older J Ruby - gem "jruby-openssl", "< 0.10.0" - # Pin child-process on older J Ruby - gem "childprocess", "< 1.0.0" - else - gem "jruby-openssl" - end -end +gem "jruby-openssl", platforms: [:jruby] eval File.read('Gemfile-custom') if File.exist?('Gemfile-custom') diff --git a/rspec-mocks.gemspec b/rspec-mocks.gemspec index d81ad29e4..900735aec 100644 --- a/rspec-mocks.gemspec +++ b/rspec-mocks.gemspec @@ -45,7 +45,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency "diff-lcs", ">= 1.2.0", "< 2.0" - s.add_development_dependency 'rake', '> 10.0.0' + s.add_development_dependency 'rake', '> 12.3.2' s.add_development_dependency 'cucumber', '~> 1.3.15' s.add_development_dependency 'aruba', '~> 0.14.10' s.add_development_dependency 'minitest', '~> 5.2' From 0bf0581e4000abda519a702d17bccc5e5a2b4382 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 14:45:39 +0300 Subject: [PATCH 04/26] Remove feature support checks from specs --- spec/rspec/mocks/and_yield_spec.rb | 2 +- spec/rspec/mocks/double_spec.rb | 12 ++- .../expected_arg_verification_spec.rb | 36 ++++---- .../instance_double_with_class_loaded_spec.rb | 88 +++++++++---------- spec/spec_helper.rb | 3 - spec/support/doubled_classes.rb | 13 +-- 6 files changed, 68 insertions(+), 86 deletions(-) diff --git a/spec/rspec/mocks/and_yield_spec.rb b/spec/rspec/mocks/and_yield_spec.rb index 75cc4a979..20bbe3fd6 100644 --- a/spec/rspec/mocks/and_yield_spec.rb +++ b/spec/rspec/mocks/and_yield_spec.rb @@ -99,7 +99,7 @@ verify yielded_arg end - context "that are optional", :if => RSpec::Support::RubyFeatures.optional_and_splat_args_supported? do + context "that are optional" do it "yields the default argument when the argument is not given" do pending "Not sure how to achieve this yet. See rspec/rspec-mocks#714 and rspec/rspec-support#85." default_arg = Object.new diff --git a/spec/rspec/mocks/double_spec.rb b/spec/rspec/mocks/double_spec.rb index ee4629c0a..973aa0102 100644 --- a/spec/rspec/mocks/double_spec.rb +++ b/spec/rspec/mocks/double_spec.rb @@ -524,13 +524,11 @@ def initialize(amount, units) }.to fail_with "# yielded |\"wha\", \"zup\"| to block with arity of 1" end - if kw_args_supported? - it 'fails when calling yielding method with invalid kw args' do - expect(@double).to receive(:yield_back).and_yield(:x => 1, :y => 2) - expect { - eval("@double.yield_back { |x: 1| }") - }.to fail_with '# yielded |{:x=>1, :y=>2}| to block with optional keyword args (:x)' - end + it 'fails when calling yielding method with invalid kw args' do + expect(@double).to receive(:yield_back).and_yield(:x => 1, :y => 2) + expect { + eval("@double.yield_back { |x: 1| }") + }.to fail_with '# yielded |{:x=>1, :y=>2}| to block with optional keyword args (:x)' end it "fails when calling yielding method consecutively with wrong arity" do diff --git a/spec/rspec/mocks/verifying_doubles/expected_arg_verification_spec.rb b/spec/rspec/mocks/verifying_doubles/expected_arg_verification_spec.rb index 574d718f9..e3fd4645f 100644 --- a/spec/rspec/mocks/verifying_doubles/expected_arg_verification_spec.rb +++ b/spec/rspec/mocks/verifying_doubles/expected_arg_verification_spec.rb @@ -75,30 +75,26 @@ module Mocks }.to fail_with("Wrong number of arguments. Expected 2, got 0.") end - if RSpec::Support::RubyFeatures.required_kw_args_supported? - context "for a method with required keyword args" do - it 'covers the required args when `any_args` is last' do - expect { - expect(dbl).to receive(:kw_args_method).with(1, any_args) - }.not_to raise_error - end - - it 'does not cover the required args when there are args after `any_args`' do - expect { - # Use eval to avoid syntax error on 1.8 and 1.9 - eval("expect(dbl).to receive(:kw_args_method).with(any_args, optional_arg: 3)") - }.to fail_with("Missing required keyword arguments: required_arg") - end + context "for a method with required keyword args" do + it 'covers the required args when `any_args` is last' do + expect { + expect(dbl).to receive(:kw_args_method).with(1, any_args) + }.not_to raise_error + end + + it 'does not cover the required args when there are args after `any_args`' do + expect { + # Use eval to avoid syntax error on 1.8 and 1.9 + eval("expect(dbl).to receive(:kw_args_method).with(any_args, optional_arg: 3)") + }.to fail_with("Missing required keyword arguments: required_arg") end end end - if RSpec::Support::RubyFeatures.required_kw_args_supported? - it 'does not cover required args when `any_args` is not used' do - expect { - expect(dbl).to receive(:kw_args_method).with(anything) - }.to fail_with("Missing required keyword arguments: required_arg") - end + it 'does not cover required args when `any_args` is not used' do + expect { + expect(dbl).to receive(:kw_args_method).with(anything) + }.to fail_with("Missing required keyword arguments: required_arg") end context "when a list of args is provided" do diff --git a/spec/rspec/mocks/verifying_doubles/instance_double_with_class_loaded_spec.rb b/spec/rspec/mocks/verifying_doubles/instance_double_with_class_loaded_spec.rb index e7faf79f2..f9423b04d 100644 --- a/spec/rspec/mocks/verifying_doubles/instance_double_with_class_loaded_spec.rb +++ b/spec/rspec/mocks/verifying_doubles/instance_double_with_class_loaded_spec.rb @@ -83,62 +83,60 @@ module Mocks }.to raise_error(ArgumentError, "Wrong number of arguments. Expected 0, got 1.") end - if required_kw_args_supported? - it 'allows keyword arguments' do - o = instance_double('LoadedClass', :kw_args_method => true) - expect(o.kw_args_method(1, :required_arg => 'something')).to eq(true) - end - - context 'for a method that only accepts keyword args' do - it 'allows hash matchers like `hash_including` to be used in place of the keywords arg hash' do - o = instance_double('LoadedClass') - expect(o).to receive(:kw_args_method). - with(1, hash_including(:required_arg => 1)) - o.kw_args_method(1, :required_arg => 1) - end - - it 'allows anything matcher to be used in place of the keywords arg hash' do - o = instance_double('LoadedClass') - expect(o).to receive(:kw_args_method).with(1, anything) - o.kw_args_method(1, :required_arg => 1) - end + it 'allows keyword arguments' do + o = instance_double('LoadedClass', :kw_args_method => true) + expect(o.kw_args_method(1, :required_arg => 'something')).to eq(true) + end - it 'still checks positional arguments when matchers used for keyword args' do - o = instance_double('LoadedClass') - prevents(/Expected 1, got 3/) { - expect(o).to receive(:kw_args_method). - with(1, 2, 3, hash_including(:required_arg => 1)) - } - reset o - end + context 'for a method that only accepts keyword args' do + it 'allows hash matchers like `hash_including` to be used in place of the keywords arg hash' do + o = instance_double('LoadedClass') + expect(o).to receive(:kw_args_method). + with(1, hash_including(:required_arg => 1)) + o.kw_args_method(1, :required_arg => 1) + end - it 'does not allow matchers to be used in an actual method call' do - o = instance_double('LoadedClass') - matcher = hash_including(:required_arg => 1) - allow(o).to receive(:kw_args_method).with(1, matcher) - expect { - o.kw_args_method(1, matcher) - }.to raise_error(ArgumentError) - end + it 'allows anything matcher to be used in place of the keywords arg hash' do + o = instance_double('LoadedClass') + expect(o).to receive(:kw_args_method).with(1, anything) + o.kw_args_method(1, :required_arg => 1) end - context 'for a method that accepts a mix of optional keyword and positional args' do - it 'allows hash matchers like `hash_including` to be used in place of the keywords arg hash' do - o = instance_double('LoadedClass') - expect(o).to receive(:mixed_args_method).with(1, 2, hash_including(:optional_arg_1 => 1)) - o.mixed_args_method(1, 2, :optional_arg_1 => 1) - end + it 'still checks positional arguments when matchers used for keyword args' do + o = instance_double('LoadedClass') + prevents(/Expected 1, got 3/) { + expect(o).to receive(:kw_args_method). + with(1, 2, 3, hash_including(:required_arg => 1)) + } + reset o end - it 'checks that stubbed methods with required keyword args are ' \ - 'invoked with the required arguments' do - o = instance_double('LoadedClass', :kw_args_method => true) + it 'does not allow matchers to be used in an actual method call' do + o = instance_double('LoadedClass') + matcher = hash_including(:required_arg => 1) + allow(o).to receive(:kw_args_method).with(1, matcher) expect { - o.kw_args_method(:optional_arg => 'something') + o.kw_args_method(1, matcher) }.to raise_error(ArgumentError) end end + context 'for a method that accepts a mix of optional keyword and positional args' do + it 'allows hash matchers like `hash_including` to be used in place of the keywords arg hash' do + o = instance_double('LoadedClass') + expect(o).to receive(:mixed_args_method).with(1, 2, hash_including(:optional_arg_1 => 1)) + o.mixed_args_method(1, 2, :optional_arg_1 => 1) + end + end + + it 'checks that stubbed methods with required keyword args are ' \ + 'invoked with the required arguments' do + o = instance_double('LoadedClass', :kw_args_method => true) + expect { + o.kw_args_method(:optional_arg => 'something') + }.to raise_error(ArgumentError) + end + it 'validates `with` args against the method signature when stubbing a method' do dbl = instance_double(LoadedClass) prevents(/Wrong number of arguments. Expected 2, got 3./) { diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7388670af..7097955e4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,4 @@ require 'rspec/support/spec' -require 'rspec/support/ruby_features' RSpec::Support::Spec.setup_simplecov do minimum_coverage 93 @@ -105,8 +104,6 @@ def self.fake_matcher_description config.include VerifyAndResetHelpers config.include MatcherHelpers config.include VerificationHelpers - config.extend RSpec::Support::RubyFeatures - config.include RSpec::Support::RubyFeatures config.define_derived_metadata :ordered_and_vague_counts_unsupported do |meta| meta[:pending] = "`.ordered` combined with a vague count (e.g. `at_least` or `at_most`) is not yet supported (see #713)" diff --git a/spec/support/doubled_classes.rb b/spec/support/doubled_classes.rb index ab7d9dc72..399e7a66b 100644 --- a/spec/support/doubled_classes.rb +++ b/spec/support/doubled_classes.rb @@ -1,6 +1,4 @@ class LoadedClass - extend RSpec::Support::RubyFeatures - M = :m N = :n INSTANCE = LoadedClass.new @@ -47,15 +45,10 @@ def instance_method_with_only_defaults(_a=1, _b=2) def defined_instance_and_class_method end - if required_kw_args_supported? - # Need to eval this since it is invalid syntax on earlier rubies. - eval <<-RUBY - def kw_args_method(foo, optional_arg:'hello', required_arg:) - end + def kw_args_method(foo, optional_arg:'hello', required_arg:) + end - def mixed_args_method(foo, bar, optional_arg_1:1, optional_arg_2:2) - end - RUBY + def mixed_args_method(foo, bar, optional_arg_1:1, optional_arg_2:2) end def send(*) From 5e064b8a0acca120d5aca991966935894fc43cd8 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 14:50:02 +0300 Subject: [PATCH 05/26] Move require to where it's used --- lib/rspec/mocks.rb | 1 - lib/rspec/mocks/error_generator.rb | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rspec/mocks.rb b/lib/rspec/mocks.rb index 297779e5a..32e56e077 100644 --- a/lib/rspec/mocks.rb +++ b/lib/rspec/mocks.rb @@ -1,7 +1,6 @@ require 'rspec/support' RSpec::Support.require_rspec_support 'caller_filter' RSpec::Support.require_rspec_support 'warnings' -RSpec::Support.require_rspec_support 'ruby_features' RSpec::Support.define_optimized_require_for_rspec(:mocks) { |f| require_relative f } diff --git a/lib/rspec/mocks/error_generator.rb b/lib/rspec/mocks/error_generator.rb index 9bf0984f3..8d4cc5eaa 100644 --- a/lib/rspec/mocks/error_generator.rb +++ b/lib/rspec/mocks/error_generator.rb @@ -1,4 +1,5 @@ RSpec::Support.require_rspec_support "object_formatter" +RSpec::Support.require_rspec_support 'ruby_features' module RSpec module Mocks From 8372c1d0caf0f97b90b5b929c15db33e3bbbd0a3 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 15:10:08 +0300 Subject: [PATCH 06/26] Remove old Ruby specific workarounds --- lib/rspec/mocks/any_instance/proxy.rb | 10 +- lib/rspec/mocks/instance_method_stasher.rb | 119 ++++-------------- lib/rspec/mocks/method_double.rb | 6 +- lib/rspec/mocks/method_reference.rb | 16 +-- lib/rspec/mocks/mutate_const.rb | 1 - lib/rspec/mocks/proxy.rb | 13 -- lib/rspec/mocks/syntax.rb | 14 --- lib/rspec/mocks/verifying_double.rb | 2 +- spec/rspec/mocks/and_call_original_spec.rb | 2 +- spec/rspec/mocks/any_instance_spec.rb | 2 +- spec/rspec/mocks/diffing_spec.rb | 15 +-- spec/rspec/mocks/double_spec.rb | 10 +- .../mocks/instance_method_stasher_spec.rb | 4 +- spec/rspec/mocks/null_object_double_spec.rb | 8 +- spec/rspec/mocks/partial_double_spec.rb | 9 +- spec/rspec/mocks/should_syntax_spec.rb | 2 +- spec/rspec/mocks/syntax_spec.rb | 7 -- spec/rspec/mocks/to_ary_spec.rb | 2 +- ...class_double_with_class_not_loaded_spec.rb | 2 +- .../instance_double_with_class_loaded_spec.rb | 16 +-- spec/rspec/mocks_spec.rb | 22 +--- 21 files changed, 46 insertions(+), 236 deletions(-) delete mode 100644 spec/rspec/mocks/syntax_spec.rb diff --git a/lib/rspec/mocks/any_instance/proxy.rb b/lib/rspec/mocks/any_instance/proxy.rb index fc66d392a..511048acd 100644 --- a/lib/rspec/mocks/any_instance/proxy.rb +++ b/lib/rspec/mocks/any_instance/proxy.rb @@ -96,14 +96,8 @@ def initialize(targets) @targets = targets end - if RUBY_VERSION.to_f > 1.8 - def respond_to_missing?(method_name, include_private=false) - super || @targets.first.respond_to?(method_name, include_private) - end - else - def respond_to?(method_name, include_private=false) - super || @targets.first.respond_to?(method_name, include_private) - end + def respond_to_missing?(method_name, include_private=false) + super || @targets.first.respond_to?(method_name, include_private) end def method_missing(*args, &block) diff --git a/lib/rspec/mocks/instance_method_stasher.rb b/lib/rspec/mocks/instance_method_stasher.rb index 12edec2fa..08719db4f 100644 --- a/lib/rspec/mocks/instance_method_stasher.rb +++ b/lib/rspec/mocks/instance_method_stasher.rb @@ -8,90 +8,40 @@ def initialize(object, method) @klass = (class << object; self; end) @original_method = nil - @method_is_stashed = false end attr_reader :original_method - if RUBY_VERSION.to_f < 1.9 - # @private - def method_is_stashed? - @method_is_stashed - end - - # @private - def stash - return if !method_defined_directly_on_klass? || @method_is_stashed - - @klass.__send__(:alias_method, stashed_method_name, @method) - @method_is_stashed = true - end - - # @private - def stashed_method_name - "obfuscated_by_rspec_mocks__#{@method}" - end - - # @private - def restore - return unless @method_is_stashed + # @private + def method_is_stashed? + !!@original_method + end - if @klass.__send__(:method_defined?, @method) - @klass.__send__(:undef_method, @method) - end - @klass.__send__(:alias_method, @method, stashed_method_name) - @klass.__send__(:remove_method, stashed_method_name) - @method_is_stashed = false - end - else + # @private + def stash + return unless method_defined_directly_on_klass? + @original_method ||= ::RSpec::Support.method_handle_for(@object, @method) + @klass.__send__(:undef_method, @method) + end - # @private - def method_is_stashed? - !!@original_method - end + # @private + def restore + return unless @original_method - # @private - def stash - return unless method_defined_directly_on_klass? - @original_method ||= ::RSpec::Support.method_handle_for(@object, @method) + if @klass.__send__(:method_defined?, @method) @klass.__send__(:undef_method, @method) end - # @private - def restore - return unless @original_method - - if @klass.__send__(:method_defined?, @method) - @klass.__send__(:undef_method, @method) - end - - handle_restoration_failures do - @klass.__send__(:define_method, @method, @original_method) - end - - @original_method = nil + handle_restoration_failures do + @klass.__send__(:define_method, @method, @original_method) end + + @original_method = nil end - if RUBY_DESCRIPTION.include?('2.0.0p247') || RUBY_DESCRIPTION.include?('2.0.0p195') - # ruby 2.0.0-p247 and 2.0.0-p195 both have a bug that we can't work around :(. - # https://bugs.ruby-lang.org/issues/8686 - def handle_restoration_failures - yield - rescue TypeError - RSpec.warn_with( - "RSpec failed to properly restore a partial double (#{@object.inspect}) " \ - "to its original state due to a known bug in MRI 2.0.0-p195 & p247 " \ - "(https://bugs.ruby-lang.org/issues/8686). This object may remain " \ - "screwed up for the rest of this process. Please upgrade to 2.0.0-p353 or above.", - :call_site => nil, :use_spec_location_as_call_site => true - ) - end - else - def handle_restoration_failures - # No known reasons for restoration to fail on other rubies. - yield - end + def handle_restoration_failures + # No known reasons for restoration to fail on other rubies. + yield end private @@ -109,36 +59,11 @@ def method_defined_on_klass?(klass=@klass) def method_owned_by_klass? owner = @klass.instance_method(@method).owner - # On Ruby 2.0.0+ the owner of a method on a class which has been + # The owner of a method on a class which has been # `prepend`ed may actually be an instance, e.g. # `#`, rather than the expected `MyClass`. owner = owner.class unless Module === owner - # On some 1.9s (e.g. rubinius) aliased methods - # can report the wrong owner. Example: - # class MyClass - # class << self - # alias alternate_new new - # end - # end - # - # MyClass.owner(:alternate_new) returns `Class` when incorrect, - # but we need to consider the owner to be `MyClass` because - # it is not actually available on `Class` but is on `MyClass`. - # Hence, we verify that the owner actually has the method defined. - # If the given owner does not have the method defined, we assume - # that the method is actually owned by @klass. - # - # On 1.8, aliased methods can also report the wrong owner. Example: - # module M - # def a; end - # module_function :a - # alias b a - # module_function :b - # end - # The owner of M.b is the raw Module object, instead of the expected - # singleton class of the module - return true if RUBY_VERSION < '1.9' && owner == @object owner == @klass || !(method_defined_on_klass?(owner)) end end diff --git a/lib/rspec/mocks/method_double.rb b/lib/rspec/mocks/method_double.rb index 8d1f892af..85045ddea 100644 --- a/lib/rspec/mocks/method_double.rb +++ b/lib/rspec/mocks/method_double.rb @@ -258,11 +258,7 @@ def remove_method_from_definition_target # # Note: we could avoid rescuing this by checking # `definition_target.instance_method(@method_name).owner == definition_target`, - # saving us from the cost of the expensive exception, but this error is - # extremely rare (it was discovered on 2014-12-30, only happens on - # RUBY_VERSION < 2.0 and our spec suite only hits this condition once), - # so we'd rather avoid the cost of that check for every method double, - # and risk the rare situation where this exception will get raised. + # but this error is extremely rare, so we'd rather rescue this exception. RSpec.warn_with( "WARNING: RSpec could not fully restore #{@object.inspect}." \ "#{@method_name}, possibly because the method has been redefined " \ diff --git a/lib/rspec/mocks/method_reference.rb b/lib/rspec/mocks/method_reference.rb index 026c2c07d..ee230fd23 100644 --- a/lib/rspec/mocks/method_reference.rb +++ b/lib/rspec/mocks/method_reference.rb @@ -124,20 +124,8 @@ def method_implemented?(mod) # definition of "implemented". However, it's the best we can do. alias method_defined? method_implemented? - # works around the fact that repeated calls for method parameters will - # falsely return empty arrays on JRuby in certain circumstances, this - # is necessary here because we can't dup/clone UnboundMethods. - # - # This is necessary due to a bug in JRuby prior to 1.7.5 fixed in: - # https://github.com/jruby/jruby/commit/99a0613fe29935150d76a9a1ee4cf2b4f63f4a27 - if RUBY_PLATFORM == 'java' && RSpec::Support::ComparableVersion.new(JRUBY_VERSION) < '1.7.5' - def find_method(mod) - mod.dup.instance_method(@method_name) - end - else - def find_method(mod) - mod.instance_method(@method_name) - end + def find_method(mod) + mod.instance_method(@method_name) end def visibility_from(mod) diff --git a/lib/rspec/mocks/mutate_const.rb b/lib/rspec/mocks/mutate_const.rb index 69a93037d..5bc107306 100644 --- a/lib/rspec/mocks/mutate_const.rb +++ b/lib/rspec/mocks/mutate_const.rb @@ -248,7 +248,6 @@ def verify_constants_to_transfer! end if Array === @transfer_nested_constants - @transfer_nested_constants = @transfer_nested_constants.map(&:to_s) if RUBY_VERSION == '1.8.7' undefined_constants = @transfer_nested_constants - constants_defined_on(@original_value) if undefined_constants.any? diff --git a/lib/rspec/mocks/proxy.rb b/lib/rspec/mocks/proxy.rb index 699ef6a3e..8a5f33efd 100644 --- a/lib/rspec/mocks/proxy.rb +++ b/lib/rspec/mocks/proxy.rb @@ -393,19 +393,6 @@ def original_method_handle_for(message) return super unless unbound_method unbound_method.bind(object) - # :nocov: - rescue TypeError - if RUBY_VERSION == '1.8.7' - # In MRI 1.8.7, a singleton method on a class cannot be rebound to its subclass - if unbound_method && unbound_method.owner.ancestors.first != unbound_method.owner - # This is a singleton method; we can't do anything with it - # But we can work around this using a different implementation - double = method_double_from_ancestor_for(message) - return object.method(double.method_stasher.stashed_method_name) - end - end - raise - # :nocov: end protected diff --git a/lib/rspec/mocks/syntax.rb b/lib/rspec/mocks/syntax.rb index 6305ab961..8d8625ea0 100644 --- a/lib/rspec/mocks/syntax.rb +++ b/lib/rspec/mocks/syntax.rb @@ -179,20 +179,6 @@ def self.expect_enabled?(syntax_host=::RSpec::Mocks::ExampleMethods) # @api private # Determines where the methods like `should_receive`, and `stub` are added. def self.default_should_syntax_host - # JRuby 1.7.4 introduces a regression whereby `defined?(::BasicObject) => nil` - # yet `BasicObject` still exists and patching onto ::Object breaks things - # e.g. SimpleDelegator expectations won't work - # - # See: https://github.com/jruby/jruby/issues/814 - if defined?(JRUBY_VERSION) && JRUBY_VERSION == '1.7.4' && RUBY_VERSION.to_f > 1.8 - return ::BasicObject - end - - # On 1.8.7, Object.ancestors.last == Kernel but - # things blow up if we include `RSpec::Mocks::Methods` - # into Kernel...not sure why. - return Object unless defined?(::BasicObject) - # MacRuby has BasicObject but it's not the root class. return Object unless Object.ancestors.last == ::BasicObject diff --git a/lib/rspec/mocks/verifying_double.rb b/lib/rspec/mocks/verifying_double.rb index 39723b9fc..040fb4531 100644 --- a/lib/rspec/mocks/verifying_double.rb +++ b/lib/rspec/mocks/verifying_double.rb @@ -12,7 +12,7 @@ def respond_to?(message, include_private=false) case method_ref.visibility when :public then true when :private then include_private - when :protected then include_private || RUBY_VERSION.to_f < 2.0 + when :protected then include_private else !method_ref.unimplemented? end end diff --git a/spec/rspec/mocks/and_call_original_spec.rb b/spec/rspec/mocks/and_call_original_spec.rb index 8bb20d563..f4dee7cbc 100644 --- a/spec/rspec/mocks/and_call_original_spec.rb +++ b/spec/rspec/mocks/and_call_original_spec.rb @@ -80,7 +80,7 @@ def instance.foo; :bar; end expect(instance.foo).to eq(:bar) end - it 'works for SimpleDelegator subclasses', :if => (RUBY_VERSION.to_f > 1.8) do + it 'works for SimpleDelegator subclasses' do inst = Class.new(SimpleDelegator).new(1) def inst.foo; :bar; end expect(inst).to receive(:foo).and_call_original diff --git a/spec/rspec/mocks/any_instance_spec.rb b/spec/rspec/mocks/any_instance_spec.rb index 1f63bf4ce..ca4ca232c 100644 --- a/spec/rspec/mocks/any_instance_spec.rb +++ b/spec/rspec/mocks/any_instance_spec.rb @@ -780,7 +780,7 @@ def foo; end klazz.new.foo end - it 'works with a SimpleDelegator subclass', :if => (RUBY_VERSION.to_f > 1.8) do + it 'works with a SimpleDelegator subclass' do klazz = Class.new(SimpleDelegator) do def foo; end end diff --git a/spec/rspec/mocks/diffing_spec.rb b/spec/rspec/mocks/diffing_spec.rb index 3b1f91edf..97a023dd0 100644 --- a/spec/rspec/mocks/diffing_spec.rb +++ b/spec/rspec/mocks/diffing_spec.rb @@ -83,19 +83,8 @@ end end - if RUBY_VERSION.to_f < 1.9 - # Ruby 1.8 hashes are not ordered, but `#inspect` on a particular unchanged - # hash instance should return consistent output. However, on Travis that does - # not always seem to be true and we have no idea why. Somehow, the travis build - # has occasionally failed due to the output ordering varying between `inspect` - # calls to the same hash. This regex allows us to work around that. - def hash_regex_inspect(hash) - "\\{(#{hash.map { |key, value| "#{key.inspect}=>#{value.inspect}.*" }.join "|"}){#{hash.size}}\\}" - end - else - def hash_regex_inspect(hash) - Regexp.escape(hash.inspect) - end + def hash_regex_inspect(hash) + Regexp.escape(hash.inspect) end it "prints a diff with array args" do diff --git a/spec/rspec/mocks/double_spec.rb b/spec/rspec/mocks/double_spec.rb index 973aa0102..58ba104e7 100644 --- a/spec/rspec/mocks/double_spec.rb +++ b/spec/rspec/mocks/double_spec.rb @@ -304,13 +304,7 @@ def @double.method_with_default_argument(_={}); end end it "responds to to_a as a null object" do - if RUBY_VERSION.to_f > 1.8 - expect(@double.as_null_object.to_a).to eq nil - else - with_isolated_stderr do - expect(@double.as_null_object.to_a).to eq [@double] - end - end + expect(@double.as_null_object.to_a).to eq nil end it "passes proc to expectation block without an argument" do @@ -724,7 +718,7 @@ def add_call end end - describe "#to_str", :unless => RUBY_VERSION == '1.9.2' do + describe "#to_str" do it "should not respond to #to_str to avoid being coerced to strings by the runtime" do dbl = double("Foo") expect { dbl.to_str }.to raise_error( diff --git a/spec/rspec/mocks/instance_method_stasher_spec.rb b/spec/rspec/mocks/instance_method_stasher_spec.rb index e0614cc0f..d86e1a590 100644 --- a/spec/rspec/mocks/instance_method_stasher_spec.rb +++ b/spec/rspec/mocks/instance_method_stasher_spec.rb @@ -55,7 +55,7 @@ def obj.hello; :overridden_hello; end; expect(obj.hello).to eql :overridden_hello end - it "does not unnecessarily create obfuscated aliased methods", :if => (RUBY_VERSION.to_f > 1.8) do + it "does not unnecessarily create obfuscated aliased methods" do obj = Object.new def obj.hello; :hello_defined_on_singleton_class; end; @@ -64,7 +64,7 @@ def obj.hello; :hello_defined_on_singleton_class; end; expect(obj.methods.grep(/rspec/)).to eq([]) end - it "undefines the original method", :if => (RUBY_VERSION.to_f > 1.8) do + it "undefines the original method" do obj = Object.new def obj.hello; :hello_defined_on_singleton_class; end; diff --git a/spec/rspec/mocks/null_object_double_spec.rb b/spec/rspec/mocks/null_object_double_spec.rb index 2b62ef45f..0a8a06201 100644 --- a/spec/rspec/mocks/null_object_double_spec.rb +++ b/spec/rspec/mocks/null_object_double_spec.rb @@ -15,13 +15,7 @@ module Mocks end it "raises an error when interpolated in a string as an integer" do - # Not sure why, but 1.9.2 (but not JRuby --1.9) raises a different - # error than 1.8.7 and 1.9.3... - expected_error = (RUBY_VERSION == '1.9.2' && RUBY_PLATFORM !~ /java/) ? - RSpec::Mocks::MockExpectationError : - TypeError - - expect { "%i" % @double }.to raise_error(expected_error) + expect { "%i" % @double }.to raise_error(TypeError) end end diff --git a/spec/rspec/mocks/partial_double_spec.rb b/spec/rspec/mocks/partial_double_spec.rb index 39b929f58..7bda7a1f1 100644 --- a/spec/rspec/mocks/partial_double_spec.rb +++ b/spec/rspec/mocks/partial_double_spec.rb @@ -93,7 +93,6 @@ def call(name) end it 'allows a class and a subclass to both be stubbed' do - pending "Does not work on 1.8.7 due to singleton method restrictions" if RUBY_VERSION == "1.8.7" && RSpec::Support::Ruby.mri? the_klass = Class.new the_subklass = Class.new(the_klass) @@ -182,13 +181,7 @@ def read_file(file) end def expect_output_warning_on_ruby_lt_2 - if RUBY_VERSION >= '2.0' - yield - else - expect { yield }.to output(a_string_including( - "RSpec could not fully restore", file_1.inspect, "write" - )).to_stderr - end + yield end it "allows `write` to be stubbed and reset", :unless => Support::Ruby.jruby? do diff --git a/spec/rspec/mocks/should_syntax_spec.rb b/spec/rspec/mocks/should_syntax_spec.rb index 255f5be21..eb00597ed 100644 --- a/spec/rspec/mocks/should_syntax_spec.rb +++ b/spec/rspec/mocks/should_syntax_spec.rb @@ -245,7 +245,7 @@ def use_rspec_mocks expect { verify_all }.to fail end - it 'can get method objects for the fluent interface', :if => RUBY_VERSION.to_f > 1.8 do + it 'can get method objects for the fluent interface' do and_return = klass.any_instance.stub(:foo).method(:and_return) and_return.call(23) diff --git a/spec/rspec/mocks/syntax_spec.rb b/spec/rspec/mocks/syntax_spec.rb deleted file mode 100644 index ad84af834..000000000 --- a/spec/rspec/mocks/syntax_spec.rb +++ /dev/null @@ -1,7 +0,0 @@ -module RSpec - RSpec.describe Mocks do - it "does not inadvertently define BasicObject on 1.8", :if => RUBY_VERSION.to_f < 1.9 do - expect(defined?(::BasicObject)).to be nil - end - end -end diff --git a/spec/rspec/mocks/to_ary_spec.rb b/spec/rspec/mocks/to_ary_spec.rb index 49984d7d7..f2d33baf6 100644 --- a/spec/rspec/mocks/to_ary_spec.rb +++ b/spec/rspec/mocks/to_ary_spec.rb @@ -41,7 +41,7 @@ expect(obj).not_to respond_to(:to_ary) end - it "doesn't respond to to_a", :if => (RUBY_VERSION.to_f > 1.8) do + it "doesn't respond to to_a" do expect(obj).not_to respond_to(:to_a) end diff --git a/spec/rspec/mocks/verifying_doubles/class_double_with_class_not_loaded_spec.rb b/spec/rspec/mocks/verifying_doubles/class_double_with_class_not_loaded_spec.rb index a41ee406e..aea0b76b6 100644 --- a/spec/rspec/mocks/verifying_doubles/class_double_with_class_not_loaded_spec.rb +++ b/spec/rspec/mocks/verifying_doubles/class_double_with_class_not_loaded_spec.rb @@ -22,7 +22,7 @@ module Mocks expect(o.undefined_instance_method(:arg)).to eq(1) end - specify "trying to raise a class_double raises a TypeError", :unless => RUBY_VERSION == '1.9.2' do + specify "trying to raise a class_double raises a TypeError" do subject = Object.new class_double("StubbedError").as_stubbed_const allow(subject).to receive(:some_method).and_raise(StubbedError) diff --git a/spec/rspec/mocks/verifying_doubles/instance_double_with_class_loaded_spec.rb b/spec/rspec/mocks/verifying_doubles/instance_double_with_class_loaded_spec.rb index f9423b04d..2929629a9 100644 --- a/spec/rspec/mocks/verifying_doubles/instance_double_with_class_loaded_spec.rb +++ b/spec/rspec/mocks/verifying_doubles/instance_double_with_class_loaded_spec.rb @@ -211,18 +211,10 @@ module Mocks expect(obj.respond_to?(:defined_private_method, false)).to be false end - if RUBY_VERSION.to_f < 2.0 # respond_to?(:protected_method) changed behavior in Ruby 2.0. - it 'reports that it responds to protected methods' do - expect(obj.respond_to?(:defined_protected_method)).to be true - expect(obj.respond_to?(:defined_protected_method, true)).to be true - expect(obj.respond_to?(:defined_protected_method, false)).to be true - end - else - it 'reports that it responds to protected methods when the appropriate arg is passed' do - expect(obj.respond_to?(:defined_protected_method)).to be false - expect(obj.respond_to?(:defined_protected_method, true)).to be true - expect(obj.respond_to?(:defined_protected_method, false)).to be false - end + it 'reports that it responds to protected methods when the appropriate arg is passed' do + expect(obj.respond_to?(:defined_protected_method)).to be false + expect(obj.respond_to?(:defined_protected_method, true)).to be true + expect(obj.respond_to?(:defined_protected_method, false)).to be false end end end diff --git a/spec/rspec/mocks_spec.rb b/spec/rspec/mocks_spec.rb index 771431e63..e9c9a013a 100644 --- a/spec/rspec/mocks_spec.rb +++ b/spec/rspec/mocks_spec.rb @@ -12,31 +12,11 @@ 'require "rspec/mocks/any_instance"' ] - # On 1.9.2 we load securerandom to get around the lack of `BasicObject#__id__. - # Loading securerandom loads many other stdlibs it depends on. Rather than - # declaring it (and all the stdlibs it loads) as allowed, it's easier just - # to prevent the loading of securerandom by faking out `BasicObject#__id__ - lib_preamble.unshift "class BasicObject; def __id__; end; end" if RUBY_VERSION == '1.9.2' - it_behaves_like 'library wide checks', 'rspec-mocks', :preamble_for_lib => lib_preamble, :allowed_loaded_feature_regexps => [ /rbconfig/ # loaded by rspec-support - ] do - - if RSpec::Support::Ruby.jruby? && JRUBY_VERSION =~ /9\.1\.7\.0/ - before(:example, :description => /spec files/) do - pending "JRuby 9.1.7.0 currently generates a circular warning which" \ - " is unrelated to our suite." - end - end - - if RUBY_VERSION == '1.9.2' - before(:example, :description => /spec files/) do - pending "Loading psych and syck on 1.9.2 (as our test suite does) triggers warnings" - end - end - end + ] describe ".verify" do it "delegates to the space" do From 3c5f54b90e9e262aefedeac962787c3a616c0fa4 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 17:40:16 +0300 Subject: [PATCH 07/26] Get rid of old Ruby workarounds --- lib/rspec/mocks/any_instance/recorder.rb | 1 + lib/rspec/mocks/instance_method_stasher.rb | 7 ++++++- lib/rspec/mocks/method_double.rb | 8 +++++--- lib/rspec/mocks/method_reference.rb | 2 ++ lib/rspec/mocks/minitest_integration.rb | 2 +- lib/rspec/mocks/space.rb | 19 ++----------------- .../method_visibility_spec.rb | 3 +-- 7 files changed, 18 insertions(+), 24 deletions(-) diff --git a/lib/rspec/mocks/any_instance/recorder.rb b/lib/rspec/mocks/any_instance/recorder.rb index c5b397d67..9fe0e65f7 100644 --- a/lib/rspec/mocks/any_instance/recorder.rb +++ b/lib/rspec/mocks/any_instance/recorder.rb @@ -266,6 +266,7 @@ def observe!(method_name) def mark_invoked!(method_name) backup_method!(method_name) recorder = self + # In Ruby 2.4 and earlier, `define_method` is private @klass.__send__(:define_method, method_name) do |*_args, &_blk| invoked_instance = recorder.instance_that_received(method_name) inspect = "#<#{self.class}:#{object_id} #{instance_variables.map { |name| "#{name}=#{instance_variable_get name}" }.join(', ')}>" diff --git a/lib/rspec/mocks/instance_method_stasher.rb b/lib/rspec/mocks/instance_method_stasher.rb index 08719db4f..48dfc5a86 100644 --- a/lib/rspec/mocks/instance_method_stasher.rb +++ b/lib/rspec/mocks/instance_method_stasher.rb @@ -5,6 +5,8 @@ class InstanceMethodStasher def initialize(object, method) @object = object @method = method + # We don't want to create singleton class if it doesn't exist, + # so we don't use `object.singleton_class`. @klass = (class << object; self; end) @original_method = nil @@ -21,6 +23,7 @@ def method_is_stashed? def stash return unless method_defined_directly_on_klass? @original_method ||= ::RSpec::Support.method_handle_for(@object, @method) + # In Ruby 2.4 and earlier, `undef_method` is private @klass.__send__(:undef_method, @method) end @@ -28,11 +31,13 @@ def stash def restore return unless @original_method - if @klass.__send__(:method_defined?, @method) + if @klass.method_defined?(@method) + # In Ruby 2.4 and earlier, `undef_method` is private @klass.__send__(:undef_method, @method) end handle_restoration_failures do + # In Ruby 2.4 and earlier, `define_method` is private @klass.__send__(:define_method, @method, @original_method) end diff --git a/lib/rspec/mocks/method_double.rb b/lib/rspec/mocks/method_double.rb index 85045ddea..bcada0f65 100644 --- a/lib/rspec/mocks/method_double.rb +++ b/lib/rspec/mocks/method_double.rb @@ -21,8 +21,7 @@ def initialize(object, method_name, proxy) def original_implementation_callable # If original method is not present, uses the `method_missing` # handler of the object. This accounts for cases where the user has not - # correctly defined `respond_to?`, and also 1.8 which does not provide - # method handles for missing methods even if `respond_to?` is correct. + # correctly defined `respond_to?`. @original_implementation_callable ||= original_method || Proc.new do |*args, &block| @object.__send__(:method_missing, @method_name, *args, &block) @@ -44,6 +43,8 @@ def visibility # @private def object_singleton_class + # We can't use @object.singleton_class because this method + # creates a new singleton class if the object doesn't have one. class << @object; self; end end @@ -244,11 +245,12 @@ def usable_rspec_prepended_module def new_rspec_prepended_module RSpecPrependedModule.new.tap do |mod| - object_singleton_class.__send__ :prepend, mod + @object.singleton_class.prepend mod end end def remove_method_from_definition_target + # In Ruby 2.4 and earlier, `remove_method` is private definition_target.__send__(:remove_method, @method_name) rescue NameError # This can happen when the method has been monkeyed with by diff --git a/lib/rspec/mocks/method_reference.rb b/lib/rspec/mocks/method_reference.rb index ee230fd23..1d328fa2a 100644 --- a/lib/rspec/mocks/method_reference.rb +++ b/lib/rspec/mocks/method_reference.rb @@ -150,6 +150,8 @@ def method_implemented?(object) end def method_defined?(object) + # We don't want to create singleton class if it doesn't exist, + # so we don't use `object.singleton_class`. (class << object; self; end).method_defined?(@method_name) end diff --git a/lib/rspec/mocks/minitest_integration.rb b/lib/rspec/mocks/minitest_integration.rb index c7a7bab45..04eb40476 100644 --- a/lib/rspec/mocks/minitest_integration.rb +++ b/lib/rspec/mocks/minitest_integration.rb @@ -28,7 +28,7 @@ def after_teardown end end -Minitest::Test.send(:include, RSpec::Mocks::MinitestIntegration) +Minitest::Test.include(RSpec::Mocks::MinitestIntegration) if defined?(::Minitest::Expectation) if defined?(::RSpec::Expectations) && ::Minitest::Expectation.method_defined?(:to) diff --git a/lib/rspec/mocks/space.rb b/lib/rspec/mocks/space.rb index f5a4c2406..14cbc05a9 100644 --- a/lib/rspec/mocks/space.rb +++ b/lib/rspec/mocks/space.rb @@ -185,23 +185,8 @@ def any_instance_recorder_not_found_for(id, klass) any_instance_recorders[id] = AnyInstance::Recorder.new(klass) end - if defined?(::BasicObject) && !::BasicObject.method_defined?(:__id__) # for 1.9.2 - require 'securerandom' - - def id_for(object) - id = object.__id__ - - return id if object.equal?(::ObjectSpace._id2ref(id)) - # this suggests that object.__id__ is proxying through to some wrapped object - - object.instance_exec do - @__id_for_rspec_mocks_space ||= ::SecureRandom.uuid - end - end - else - def id_for(object) - object.__id__ - end + def id_for(object) + object.__id__ end end diff --git a/spec/rspec/mocks/verifying_doubles/method_visibility_spec.rb b/spec/rspec/mocks/verifying_doubles/method_visibility_spec.rb index e97524372..fed3fca94 100644 --- a/spec/rspec/mocks/verifying_doubles/method_visibility_spec.rb +++ b/spec/rspec/mocks/verifying_doubles/method_visibility_spec.rb @@ -111,8 +111,7 @@ def preserves_visibility(method_name, visibility) unless double.null_object? # Null object doubles use `method_missing` and so the singleton class # doesn't know what methods are defined. - singleton_class = class << double; self; end - expect(singleton_class.send("#{visibility}_method_defined?", method_name)).to be true + expect(double.singleton_class.send("#{visibility}_method_defined?", method_name)).to be true end end From 27e80fb1284c0deffa988db48e9a40d5f349282b Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 18:08:52 +0300 Subject: [PATCH 08/26] Remove unused require --- lib/rspec/mocks/method_reference.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/rspec/mocks/method_reference.rb b/lib/rspec/mocks/method_reference.rb index 1d328fa2a..023db28ca 100644 --- a/lib/rspec/mocks/method_reference.rb +++ b/lib/rspec/mocks/method_reference.rb @@ -1,5 +1,3 @@ -RSpec::Support.require_rspec_support 'comparable_version' - module RSpec module Mocks # Represents a method on an object that may or may not be defined. From 400ddcd5a952aee60da3df66b56f3455739bd459 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 17:40:16 +0300 Subject: [PATCH 09/26] Get rid of old Ruby workarounds --- lib/rspec/mocks/any_instance/recorder.rb | 1 + lib/rspec/mocks/space.rb | 2 ++ spec/rspec/mocks/space_spec.rb | 5 ++--- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/rspec/mocks/any_instance/recorder.rb b/lib/rspec/mocks/any_instance/recorder.rb index 9fe0e65f7..76e0d1d65 100644 --- a/lib/rspec/mocks/any_instance/recorder.rb +++ b/lib/rspec/mocks/any_instance/recorder.rb @@ -257,6 +257,7 @@ def observe!(method_name) @observed_methods << method_name backup_method!(method_name) recorder = self + # In Ruby 2.4 and earlier, `define_method` is private @klass.__send__(:define_method, method_name) do |*args, &blk| recorder.playback!(self, method_name) __send__(method_name, *args, &blk) diff --git a/lib/rspec/mocks/space.rb b/lib/rspec/mocks/space.rb index 14cbc05a9..d57fa9e7c 100644 --- a/lib/rspec/mocks/space.rb +++ b/lib/rspec/mocks/space.rb @@ -137,6 +137,8 @@ def any_instance_recorders_from_ancestry_of(object) # We access the ancestors through the singleton class, to avoid calling # `class` in case `class` has been stubbed. + # We can't use `object.singleton_class` here, as this method creates + # a new singleton class for the object if the object doesn't have one. (class << object; ancestors; end).map do |klass| any_instance_recorders[klass.__id__] end.compact diff --git a/spec/rspec/mocks/space_spec.rb b/spec/rspec/mocks/space_spec.rb index d5d8f5a16..0075bc90f 100644 --- a/spec/rspec/mocks/space_spec.rb +++ b/spec/rspec/mocks/space_spec.rb @@ -1,4 +1,3 @@ - module RSpec::Mocks RSpec.describe Space do let(:space) { Space.new } @@ -19,7 +18,7 @@ module RSpec::Mocks def define_singleton_method_on_recorder_for(klass, name, &block) recorder = space.any_instance_recorder_for(klass) - (class << recorder; self; end).send(:define_method, name, &block) + recorder.singleton_class.send(:define_method, name, &block) end it 'verifies all any_instance recorders within' do @@ -39,7 +38,7 @@ def define_singleton_method_on_recorder_for(klass, name, &block) it 'does not reset the proxies (as that should be delayed until reset_all)' do proxy = space.proxy_for(dbl_1) reset = false - (class << proxy; self; end).__send__(:define_method, :reset) { reset = true } + proxy.singleton_class.__send__(:define_method, :reset) { reset = true } space.verify_all expect(reset).to eq(false) From 3635e9a916f9b84660920d6694a7b6cd326dd059 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 18:29:57 +0300 Subject: [PATCH 10/26] Add Changelog entry --- Changelog.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Changelog.md b/Changelog.md index 1afb3907b..8dced4e61 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,9 @@ +### Development (unreleased) + +Breaking Changes: + +* Ruby < 2.3 is no longer supported. (Phil Pirozhkov, #1349) + ### 3.10.0 / 2020-10-30 [Full Changelog](http://github.com/rspec/rspec-mocks/compare/v3.9.1...v3.10.0) From 9a9430d2c781c3d7402d08894aba04d206f2e923 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 18:38:53 +0300 Subject: [PATCH 11/26] Remove 1.8, 1.9 workarounds --- Gemfile | 2 +- Gemfile-custom.sample | 9 +-------- lib/rspec/mocks/object_reference.rb | 10 ++-------- lib/rspec/mocks/verifying_proxy.rb | 6 ++---- spec/rspec/mocks/argument_matchers_spec.rb | 4 +--- spec/rspec/mocks/block_return_value_spec.rb | 2 -- .../expected_arg_verification_spec.rb | 3 +-- 7 files changed, 8 insertions(+), 28 deletions(-) diff --git a/Gemfile b/Gemfile index b9144bab0..2530e2f06 100644 --- a/Gemfile +++ b/Gemfile @@ -46,4 +46,4 @@ gem 'simplecov', '~> 0.8' gem "jruby-openssl", platforms: [:jruby] -eval File.read('Gemfile-custom') if File.exist?('Gemfile-custom') +eval_gemfile 'Gemfile-custom' if File.exist?('Gemfile-custom') diff --git a/Gemfile-custom.sample b/Gemfile-custom.sample index a5017674b..e346277e0 100644 --- a/Gemfile-custom.sample +++ b/Gemfile-custom.sample @@ -3,17 +3,10 @@ group :development do gem 'relish', '~> 0.6.0' gem 'guard-rspec', '~> 1.2.1' gem 'growl', '1.0.3' - gem 'spork', '0.9.0' platform :mri do gem 'rb-fsevent', '~> 0.9.0' gem 'ruby-prof', '~> 0.10.0' - - case RUBY_VERSION - when /^1.8/ - gem 'ruby-debug' - when /^1.9/ - gem 'debugger' - end + gem 'pry-byebug' end end diff --git a/lib/rspec/mocks/object_reference.rb b/lib/rspec/mocks/object_reference.rb index cce2c3313..6e9a8e802 100644 --- a/lib/rspec/mocks/object_reference.rb +++ b/lib/rspec/mocks/object_reference.rb @@ -27,14 +27,8 @@ def self.for(object_module_or_name, allow_direct_object_refs=false) end end - if Module.new.name.nil? - def self.anonymous_module?(mod) - !name_of(mod) - end - else # 1.8.7 - def self.anonymous_module?(mod) - name_of(mod) == "" - end + def self.anonymous_module?(mod) + !name_of(mod) end private_class_method :anonymous_module? diff --git a/lib/rspec/mocks/verifying_proxy.rb b/lib/rspec/mocks/verifying_proxy.rb index b39871c87..c00bf9a70 100644 --- a/lib/rspec/mocks/verifying_proxy.rb +++ b/lib/rspec/mocks/verifying_proxy.rb @@ -147,13 +147,11 @@ def message_expectation_class end def add_expectation(*args, &block) - # explict params necessary for 1.8.7 see #626 - super(*args, &block).tap { |x| x.method_reference = @method_reference } + super.tap { |x| x.method_reference = @method_reference } end def add_stub(*args, &block) - # explict params necessary for 1.8.7 see #626 - super(*args, &block).tap { |x| x.method_reference = @method_reference } + super.tap { |x| x.method_reference = @method_reference } end def proxy_method_invoked(obj, *args, &block) diff --git a/spec/rspec/mocks/argument_matchers_spec.rb b/spec/rspec/mocks/argument_matchers_spec.rb index 3df91728d..3d6e39c75 100644 --- a/spec/rspec/mocks/argument_matchers_spec.rb +++ b/spec/rspec/mocks/argument_matchers_spec.rb @@ -306,9 +306,7 @@ module Mocks it "matches against a Matcher", :reset => true do expect(a_double).to receive(:msg).with(equal(3)) - # This spec is generating warnings on 1.8.7, not sure why so - # this does with_isolated_stderr to kill them. @samphippen 3rd Jan 2013. - expect { with_isolated_stderr { a_double.msg(37) } }.to fail_including "expected: (equal 3)" + expect { a_double.msg(37) }.to fail_including "expected: (equal 3)" end it "fails when given an arbitrary object that returns false from #===", :reset => true do diff --git a/spec/rspec/mocks/block_return_value_spec.rb b/spec/rspec/mocks/block_return_value_spec.rb index f1ee7b131..b599c844f 100644 --- a/spec/rspec/mocks/block_return_value_spec.rb +++ b/spec/rspec/mocks/block_return_value_spec.rb @@ -22,8 +22,6 @@ expect(obj.foo).to eq('bar') end - # The "receives a block" part is important: 1.8.7 has a bug that reports the - # wrong arity when a block receives a block. it 'forwards all given args to the block, even when it receives a block' do obj = Object.new yielded_args = [] diff --git a/spec/rspec/mocks/verifying_doubles/expected_arg_verification_spec.rb b/spec/rspec/mocks/verifying_doubles/expected_arg_verification_spec.rb index e3fd4645f..e02b71ef5 100644 --- a/spec/rspec/mocks/verifying_doubles/expected_arg_verification_spec.rb +++ b/spec/rspec/mocks/verifying_doubles/expected_arg_verification_spec.rb @@ -84,8 +84,7 @@ module Mocks it 'does not cover the required args when there are args after `any_args`' do expect { - # Use eval to avoid syntax error on 1.8 and 1.9 - eval("expect(dbl).to receive(:kw_args_method).with(any_args, optional_arg: 3)") + expect(dbl).to receive(:kw_args_method).with(any_args, optional_arg: 3) }.to fail_with("Missing required keyword arguments: required_arg") end end From f56b1a60f379b9f53d4751b4752913afff6455da Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 18:39:12 +0300 Subject: [PATCH 12/26] Remove tricky 1.9.2 workaround See commit faa04cfb --- lib/rspec/mocks/test_double.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/rspec/mocks/test_double.rb b/lib/rspec/mocks/test_double.rb index e8bfbeb59..528d3f807 100644 --- a/lib/rspec/mocks/test_double.rb +++ b/lib/rspec/mocks/test_double.rb @@ -97,8 +97,7 @@ def method_missing(message, *args, &block) ) end - # Required wrapping doubles in an Array on Ruby 1.9.2 - raise NoMethodError if [:to_a, :to_ary].include? message + raise NoMethodError if message == :to_ary || message == :to_a proxy.raise_unexpected_message_error(message, args) end From c834a7d8d7df08098241505015ee9e770f9b6e4e Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 18:55:30 +0300 Subject: [PATCH 13/26] Remove 2.0 workarounds --- lib/rspec/mocks/method_double.rb | 3 +-- lib/rspec/mocks/proxy.rb | 6 +----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/rspec/mocks/method_double.rb b/lib/rspec/mocks/method_double.rb index bcada0f65..a8a3b9eeb 100644 --- a/lib/rspec/mocks/method_double.rb +++ b/lib/rspec/mocks/method_double.rb @@ -208,7 +208,7 @@ def raise_method_not_stubbed_error private - # In Ruby 2.0.0 and above prepend will alter the method lookup chain. + # Prepend alters the method lookup chain. # We use an object's singleton class to define method doubles upon, # however if the object has had its singleton class (as opposed to # its actual class) prepended too then the the method lookup chain @@ -218,7 +218,6 @@ def raise_method_not_stubbed_error # This code works around that by providing a mock definition target # that is either the singleton class, or if necessary, a prepended module # of our own. - # # We subclass `Module` in order to be able to easily detect our prepended module. RSpecPrependedModule = Class.new(Module) diff --git a/lib/rspec/mocks/proxy.rb b/lib/rspec/mocks/proxy.rb index 8a5f33efd..4b0e478d7 100644 --- a/lib/rspec/mocks/proxy.rb +++ b/lib/rspec/mocks/proxy.rb @@ -233,11 +233,7 @@ def visibility_for(_method_name) def self.prepended_modules_of(klass) ancestors = klass.ancestors - - # `|| 0` is necessary for Ruby 2.0, where the singleton class - # is only in the ancestor list when there are prepended modules. - singleton_index = ancestors.index(klass) || 0 - + singleton_index = ancestors.index(klass) ancestors[0, singleton_index] end From 1338a68a39b3ffefc2a62946e8c551a7925d146b Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 19:00:06 +0300 Subject: [PATCH 14/26] Embrace 2.3+ workarounds --- lib/rspec/mocks/any_instance/recorder.rb | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/lib/rspec/mocks/any_instance/recorder.rb b/lib/rspec/mocks/any_instance/recorder.rb index 76e0d1d65..60f3d907e 100644 --- a/lib/rspec/mocks/any_instance/recorder.rb +++ b/lib/rspec/mocks/any_instance/recorder.rb @@ -200,24 +200,17 @@ def restore_original_method!(method_name) remove_method method_name # A @klass can have methods implemented (see Method#owner) in @klass - # or inherited from a superclass. In ruby 2.2 and earlier, we can copy - # a method regardless of the 'owner' and restore it to @klass after - # because a call to 'super' from @klass's copied method would end up - # calling the original class's superclass's method. + # or inherited from a superclass. # - # With the commit below, available starting in 2.3.0, ruby changed - # this behavior and a call to 'super' from the method copied to @klass + # A call to 'super' from the method copied to @klass # will call @klass's superclass method, which is the original # implementer of this method! This leads to very strange errors # if @klass's copied method calls 'super', since it would end up # calling itself, the original method implemented in @klass's # superclass. # - # For ruby 2.3 and above, we need to only restore methods that - # @klass originally owned. - # - # https://github.com/ruby/ruby/commit/c8854d2ca4be9ee6946e6d17b0e17d9ef130ee81 - if RUBY_VERSION < "2.3" || backed_up_method_owner[method_name.to_sym] == self + # We need to only restore methods that @klass originally owned. + if backed_up_method_owner[method_name.to_sym] == self alias_method method_name, alias_method_name end remove_method alias_method_name From 7548ec5825d3294ae500c608e50b7103b31cb3db Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 15 Nov 2020 19:17:21 +0300 Subject: [PATCH 15/26] Remove JRuby and 1.8 workarounds --- spec/rspec/mocks/partial_double_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/rspec/mocks/partial_double_spec.rb b/spec/rspec/mocks/partial_double_spec.rb index 7bda7a1f1..152bfef81 100644 --- a/spec/rspec/mocks/partial_double_spec.rb +++ b/spec/rspec/mocks/partial_double_spec.rb @@ -184,7 +184,7 @@ def expect_output_warning_on_ruby_lt_2 yield end - it "allows `write` to be stubbed and reset", :unless => Support::Ruby.jruby? do + it "allows `write` to be stubbed and reset" do allow(file_1).to receive(:write) file_1.reopen(file_2) expect_output_warning_on_ruby_lt_2 { reset file_1 } @@ -197,7 +197,7 @@ def expect_output_warning_on_ruby_lt_2 end end - RSpec.describe "Using a partial mock on a proxy object", :if => defined?(::BasicObject) do + RSpec.describe "Using a partial mock on a proxy object" do let(:proxy_class) do Class.new(::BasicObject) do def initialize(target) From dc463abf7ea934b8c43d3597b2c7434db1dbee22 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Fri, 13 Nov 2020 00:52:49 +0300 Subject: [PATCH 16/26] Use Mutex from RSpec Support --- lib/rspec/mocks/message_expectation.rb | 2 +- lib/rspec/mocks/proxy.rb | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/rspec/mocks/message_expectation.rb b/lib/rspec/mocks/message_expectation.rb index cb5a39922..fd0cbe48a 100644 --- a/lib/rspec/mocks/message_expectation.rb +++ b/lib/rspec/mocks/message_expectation.rb @@ -1,4 +1,4 @@ -RSpec::Support.require_rspec_support 'mutex' +RSpec::Support.require_rspec_support 'reentrant_mutex' module RSpec module Mocks diff --git a/lib/rspec/mocks/proxy.rb b/lib/rspec/mocks/proxy.rb index 4b0e478d7..05bb91909 100644 --- a/lib/rspec/mocks/proxy.rb +++ b/lib/rspec/mocks/proxy.rb @@ -1,3 +1,5 @@ +RSpec::Support.require_rspec_support "reentrant_mutex" + module RSpec module Mocks # @private @@ -9,11 +11,6 @@ def ==(expectation) end end - unless defined?(Mutex) - Support.require_rspec_support 'mutex' - Mutex = Support::Mutex - end - # @private def ensure_implemented(*_args) # noop for basic proxies, see VerifyingProxy for behaviour. @@ -25,7 +22,7 @@ def initialize(object, order_group, options={}) @order_group = order_group @error_generator = ErrorGenerator.new(object) @messages_received = [] - @messages_received_mutex = Mutex.new + @messages_received_mutex = Support::Mutex.new @options = options @null_object = false @method_doubles = Hash.new { |h, k| h[k] = MethodDouble.new(@object, k, self) } From a4370fc2f49e5f4c44eaddad4207fdba30e273a1 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Mon, 23 Nov 2020 18:49:32 +0300 Subject: [PATCH 17/26] Add clarification about JRuby 9.1 workaround --- lib/rspec/mocks/test_double.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/rspec/mocks/test_double.rb b/lib/rspec/mocks/test_double.rb index 528d3f807..8ff70139c 100644 --- a/lib/rspec/mocks/test_double.rb +++ b/lib/rspec/mocks/test_double.rb @@ -84,13 +84,16 @@ def method_missing(message, *args, &block) end end + visibility = proxy.visibility_for(message) + # Defined private and protected methods will still trigger `method_missing` # when called publicly. We want ruby's method visibility error to get raised, # so we simply delegate to `super` in that case. + # return super if visibility == :private || visibility == :protected # ...well, we would delegate to `super`, but there's a JRuby # bug, so we raise our own visibility error instead: # https://github.com/jruby/jruby/issues/1398 - visibility = proxy.visibility_for(message) + # Only works without this workaround with JRuby 9.2 if visibility == :private || visibility == :protected ErrorGenerator.new(self).raise_non_public_error( message, visibility From 4cd0faf55b7a21b3258acfee09e005be7b1d7d06 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Mon, 23 Nov 2020 18:51:51 +0300 Subject: [PATCH 18/26] Remove appveyor workaround --- Gemfile | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Gemfile b/Gemfile index 2530e2f06..2b76ea42b 100644 --- a/Gemfile +++ b/Gemfile @@ -27,10 +27,6 @@ if RUBY_VERSION >= '2.4' && RUBY_ENGINE == 'ruby' gem 'rubocop', "~> 0.52.1" end -if RUBY_VERSION <= '2.3.0' && !!(RbConfig::CONFIG['host_os'] =~ /cygwin|mswin|mingw|bccwin|wince|emx/) - gem "childprocess", "< 1.0.0" -end - # Version 5.12 of minitest requires Ruby 2.4 if RUBY_VERSION < '2.4.0' gem 'minitest', '< 5.12.0' From 3af7d3af92423161f9360c4931a8dc10a668cf0e Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Mon, 23 Nov 2020 18:58:26 +0300 Subject: [PATCH 19/26] Remove JRuby workaround --- spec/rspec/mocks/any_instance_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/rspec/mocks/any_instance_spec.rb b/spec/rspec/mocks/any_instance_spec.rb index ca4ca232c..d8b366444 100644 --- a/spec/rspec/mocks/any_instance_spec.rb +++ b/spec/rspec/mocks/any_instance_spec.rb @@ -1239,8 +1239,6 @@ def call(*args) context 'when used in conjunction with a `dup`' do it "doesn't cause an infinite loop" do - skip "This intermittently fails on JRuby" if RUBY_PLATFORM == 'java' - allow_any_instance_of(Object).to receive(:some_method) o = Object.new o.some_method From 54e44a971f9acba604636b33365687d64141e502 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Mon, 23 Nov 2020 21:58:46 +0300 Subject: [PATCH 20/26] Remove handle_restoration_failures --- lib/rspec/mocks/instance_method_stasher.rb | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/rspec/mocks/instance_method_stasher.rb b/lib/rspec/mocks/instance_method_stasher.rb index 48dfc5a86..ceedb00d7 100644 --- a/lib/rspec/mocks/instance_method_stasher.rb +++ b/lib/rspec/mocks/instance_method_stasher.rb @@ -36,19 +36,12 @@ def restore @klass.__send__(:undef_method, @method) end - handle_restoration_failures do - # In Ruby 2.4 and earlier, `define_method` is private - @klass.__send__(:define_method, @method, @original_method) - end + # In Ruby 2.4 and earlier, `define_method` is private + @klass.__send__(:define_method, @method, @original_method) @original_method = nil end - def handle_restoration_failures - # No known reasons for restoration to fail on other rubies. - yield - end - private # @private From 3bc978094f8dbb1cf9d1a746b47a8d47f2d4baec Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Mon, 23 Nov 2020 22:00:30 +0300 Subject: [PATCH 21/26] Remove expect_output_warning_on_ruby_lt_2 --- spec/rspec/mocks/partial_double_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/spec/rspec/mocks/partial_double_spec.rb b/spec/rspec/mocks/partial_double_spec.rb index 152bfef81..d47caaff7 100644 --- a/spec/rspec/mocks/partial_double_spec.rb +++ b/spec/rspec/mocks/partial_double_spec.rb @@ -180,14 +180,10 @@ def read_file(file) file_2.close end - def expect_output_warning_on_ruby_lt_2 - yield - end - it "allows `write` to be stubbed and reset" do allow(file_1).to receive(:write) file_1.reopen(file_2) - expect_output_warning_on_ruby_lt_2 { reset file_1 } + reset file_1 expect { # Writing to a file that's been reopened to a 2nd file writes to both files. From ea500add9dce050ddf6990909ba4262f530a8294 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Mon, 23 Nov 2020 22:11:52 +0300 Subject: [PATCH 22/26] Relax, BasicObject is defined --- lib/rspec/mocks/syntax.rb | 195 ++++++++++++------------- spec/rspec/mocks/any_instance_spec.rb | 2 +- spec/rspec/mocks/configuration_spec.rb | 4 +- spec/rspec/mocks/stub_spec.rb | 2 +- 4 files changed, 96 insertions(+), 107 deletions(-) diff --git a/lib/rspec/mocks/syntax.rb b/lib/rspec/mocks/syntax.rb index 8d8625ea0..f728dcb6b 100644 --- a/lib/rspec/mocks/syntax.rb +++ b/lib/rspec/mocks/syntax.rb @@ -23,8 +23,8 @@ def self.warn_unless_should_configured(method_name , replacement="the new `:expe # @api private # Enables the should syntax (`dbl.stub`, `dbl.should_receive`, etc). - def self.enable_should(syntax_host=default_should_syntax_host) - @warn_about_should = false if syntax_host == default_should_syntax_host + def self.enable_should(syntax_host=::BasicObject) + @warn_about_should = false if syntax_host == ::BasicObject return if should_enabled?(syntax_host) syntax_host.class_exec do @@ -86,7 +86,7 @@ def any_instance # @api private # Disables the should syntax (`dbl.stub`, `dbl.should_receive`, etc). - def self.disable_should(syntax_host=default_should_syntax_host) + def self.disable_should(syntax_host=::BasicObject) return unless should_enabled?(syntax_host) syntax_host.class_exec do @@ -166,7 +166,7 @@ def self.disable_expect(syntax_host=::RSpec::Mocks::ExampleMethods) # @api private # Indicates whether or not the should syntax is enabled. - def self.should_enabled?(syntax_host=default_should_syntax_host) + def self.should_enabled?(syntax_host=::BasicObject) syntax_host.method_defined?(:should_receive) end @@ -175,109 +175,98 @@ def self.should_enabled?(syntax_host=default_should_syntax_host) def self.expect_enabled?(syntax_host=::RSpec::Mocks::ExampleMethods) syntax_host.method_defined?(:allow) end - - # @api private - # Determines where the methods like `should_receive`, and `stub` are added. - def self.default_should_syntax_host - # MacRuby has BasicObject but it's not the root class. - return Object unless Object.ancestors.last == ::BasicObject - - ::BasicObject - end end end end -if defined?(BasicObject) - # The legacy `:should` syntax adds the following methods directly to - # `BasicObject` so that they are available off of any object. Note, however, - # that this syntax does not always play nice with delegate/proxy objects. - # We recommend you use the non-monkeypatching `:expect` syntax instead. - # @see Class - class BasicObject - # @method should_receive - # Sets an expectation that this object should receive a message before - # the end of the example. - # - # @example - # logger = double('logger') - # thing_that_logs = ThingThatLogs.new(logger) - # logger.should_receive(:log) - # thing_that_logs.do_something_that_logs_a_message - # - # @note This is only available when you have enabled the `should` syntax. - # @see RSpec::Mocks::ExampleMethods#expect - - # @method should_not_receive - # Sets and expectation that this object should _not_ receive a message - # during this example. - # @see RSpec::Mocks::ExampleMethods#expect - - # @method stub - # Tells the object to respond to the message with the specified value. - # - # @example - # counter.stub(:count).and_return(37) - # counter.stub(:count => 37) - # counter.stub(:count) { 37 } - # - # @note This is only available when you have enabled the `should` syntax. - # @see RSpec::Mocks::ExampleMethods#allow - - # @method unstub - # Removes a stub. On a double, the object will no longer respond to - # `message`. On a real object, the original method (if it exists) is - # restored. - # - # This is rarely used, but can be useful when a stub is set up during a - # shared `before` hook for the common case, but you want to replace it - # for a special case. - # - # @note This is only available when you have enabled the `should` syntax. - - # @method stub_chain - # @overload stub_chain(method1, method2) - # @overload stub_chain("method1.method2") - # @overload stub_chain(method1, method_to_value_hash) - # - # Stubs a chain of methods. - # - # ## Warning: - # - # Chains can be arbitrarily long, which makes it quite painless to - # violate the Law of Demeter in violent ways, so you should consider any - # use of `stub_chain` a code smell. Even though not all code smells - # indicate real problems (think fluent interfaces), `stub_chain` still - # results in brittle examples. For example, if you write - # `foo.stub_chain(:bar, :baz => 37)` in a spec and then the - # implementation calls `foo.baz.bar`, the stub will not work. - # - # @example - # double.stub_chain("foo.bar") { :baz } - # double.stub_chain(:foo, :bar => :baz) - # double.stub_chain(:foo, :bar) { :baz } - # - # # Given any of ^^ these three forms ^^: - # double.foo.bar # => :baz - # - # # Common use in Rails/ActiveRecord: - # Article.stub_chain("recent.published") { [Article.new] } - # - # @note This is only available when you have enabled the `should` syntax. - # @see RSpec::Mocks::ExampleMethods#receive_message_chain - - # @method as_null_object - # Tells the object to respond to all messages. If specific stub values - # are declared, they'll work as expected. If not, the receiver is - # returned. - # - # @note This is only available when you have enabled the `should` syntax. - - # @method null_object? - # Returns true if this object has received `as_null_object` - # - # @note This is only available when you have enabled the `should` syntax. - end +# The legacy `:should` syntax adds the following methods directly to +# `BasicObject` so that they are available off of any object. Note, however, +# that this syntax does not always play nice with delegate/proxy objects. +# We recommend you use the non-monkeypatching `:expect` syntax instead. +# @see Class +class BasicObject + # @method should_receive + # Sets an expectation that this object should receive a message before + # the end of the example. + # + # @example + # logger = double('logger') + # thing_that_logs = ThingThatLogs.new(logger) + # logger.should_receive(:log) + # thing_that_logs.do_something_that_logs_a_message + # + # @note This is only available when you have enabled the `should` syntax. + # @see RSpec::Mocks::ExampleMethods#expect + + # @method should_not_receive + # Sets and expectation that this object should _not_ receive a message + # during this example. + # @see RSpec::Mocks::ExampleMethods#expect + + # @method stub + # Tells the object to respond to the message with the specified value. + # + # @example + # counter.stub(:count).and_return(37) + # counter.stub(:count => 37) + # counter.stub(:count) { 37 } + # + # @note This is only available when you have enabled the `should` syntax. + # @see RSpec::Mocks::ExampleMethods#allow + + # @method unstub + # Removes a stub. On a double, the object will no longer respond to + # `message`. On a real object, the original method (if it exists) is + # restored. + # + # This is rarely used, but can be useful when a stub is set up during a + # shared `before` hook for the common case, but you want to replace it + # for a special case. + # + # @note This is only available when you have enabled the `should` syntax. + + # @method stub_chain + # @overload stub_chain(method1, method2) + # @overload stub_chain("method1.method2") + # @overload stub_chain(method1, method_to_value_hash) + # + # Stubs a chain of methods. + # + # ## Warning: + # + # Chains can be arbitrarily long, which makes it quite painless to + # violate the Law of Demeter in violent ways, so you should consider any + # use of `stub_chain` a code smell. Even though not all code smells + # indicate real problems (think fluent interfaces), `stub_chain` still + # results in brittle examples. For example, if you write + # `foo.stub_chain(:bar, :baz => 37)` in a spec and then the + # implementation calls `foo.baz.bar`, the stub will not work. + # + # @example + # double.stub_chain("foo.bar") { :baz } + # double.stub_chain(:foo, :bar => :baz) + # double.stub_chain(:foo, :bar) { :baz } + # + # # Given any of ^^ these three forms ^^: + # double.foo.bar # => :baz + # + # # Common use in Rails/ActiveRecord: + # Article.stub_chain("recent.published") { [Article.new] } + # + # @note This is only available when you have enabled the `should` syntax. + # @see RSpec::Mocks::ExampleMethods#receive_message_chain + + # @method as_null_object + # Tells the object to respond to all messages. If specific stub values + # are declared, they'll work as expected. If not, the receiver is + # returned. + # + # @note This is only available when you have enabled the `should` syntax. + + # @method null_object? + # Returns true if this object has received `as_null_object` + # + # @note This is only available when you have enabled the `should` syntax. end # The legacy `:should` syntax adds the `any_instance` to `Class`. diff --git a/spec/rspec/mocks/any_instance_spec.rb b/spec/rspec/mocks/any_instance_spec.rb index d8b366444..b2e87d9cd 100644 --- a/spec/rspec/mocks/any_instance_spec.rb +++ b/spec/rspec/mocks/any_instance_spec.rb @@ -770,7 +770,7 @@ def inspect end end - it 'works with a BasicObject subclass that mixes in Kernel', :if => defined?(BasicObject) do + it 'works with a BasicObject subclass that mixes in Kernel' do klazz = Class.new(BasicObject) do include ::Kernel def foo; end diff --git a/spec/rspec/mocks/configuration_spec.rb b/spec/rspec/mocks/configuration_spec.rb index dd1a00f7a..0622f5b7a 100644 --- a/spec/rspec/mocks/configuration_spec.rb +++ b/spec/rspec/mocks/configuration_spec.rb @@ -64,7 +64,7 @@ def sandboxed end it 'is a no-op when configured a second time' do - expect(Syntax.default_should_syntax_host).not_to receive(:method_undefined) + expect(::BasicObject).not_to receive(:method_undefined) expect(::RSpec::Mocks::ExampleMethods).not_to receive(:method_added) configure_syntax :expect end @@ -95,7 +95,7 @@ def sandboxed end it 'is a no-op when configured a second time' do - Syntax.default_should_syntax_host.should_not_receive(:method_added) + ::BasicObject.should_not_receive(:method_added) ::RSpec::Mocks::ExampleMethods.should_not_receive(:method_undefined) configure_syntax :should end diff --git a/spec/rspec/mocks/stub_spec.rb b/spec/rspec/mocks/stub_spec.rb index c84978d3f..5fb6237b5 100644 --- a/spec/rspec/mocks/stub_spec.rb +++ b/spec/rspec/mocks/stub_spec.rb @@ -115,7 +115,7 @@ def existing_private_instance_method context "when the stubbed method is called" do it "does not call any methods on the passed args, since that could mutate them", :issue => 892 do - recorder = Class.new(defined?(::BasicObject) ? ::BasicObject : ::Object) do + recorder = Class.new(::BasicObject) do def called_methods @called_methods ||= [] end From d968590cc4421e0d61070658ad5943c0859e09c0 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sat, 28 Nov 2020 01:21:38 +0300 Subject: [PATCH 23/26] Prefer alias_method inside DSL See: https://rubystyle.guide/#alias-method https://github.com/rspec/rspec-core/pull/2787#discussion_r531794918 --- spec/rspec/mocks/verifying_doubles/naming_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/rspec/mocks/verifying_doubles/naming_spec.rb b/spec/rspec/mocks/verifying_doubles/naming_spec.rb index 0a15b28fe..926843ba3 100644 --- a/spec/rspec/mocks/verifying_doubles/naming_spec.rb +++ b/spec/rspec/mocks/verifying_doubles/naming_spec.rb @@ -45,37 +45,37 @@ def error_message_for(_) describe "instance_double" do it_behaves_like "a named verifying double", "InstanceDouble" do - alias :create_double :instance_double + alias_method :create_double, :instance_double end end describe "instance_spy" do it_behaves_like "a named verifying double", "InstanceDouble" do - alias :create_double :instance_spy + alias_method :create_double, :instance_spy end end describe "class_double" do it_behaves_like "a named verifying double", "ClassDouble" do - alias :create_double :class_double + alias_method :create_double, :class_double end end describe "class_spy" do it_behaves_like "a named verifying double", "ClassDouble" do - alias :create_double :class_spy + alias_method :create_double, :class_spy end end describe "object_double" do it_behaves_like "a named verifying double", "ObjectDouble" do - alias :create_double :object_double + alias_method :create_double, :object_double end end describe "object_spy" do it_behaves_like "a named verifying double", "ObjectDouble" do - alias :create_double :object_spy + alias_method :create_double, :object_spy end end end From b78355fca5923147fff60a3cafcab92767ac6146 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Tue, 8 Dec 2020 22:01:43 +0300 Subject: [PATCH 24/26] Reduce minumum coverage lib/rspec/mocks/order_group.rb 90.48 % --- spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7097955e4..c4124bd28 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,7 @@ require 'rspec/support/spec' RSpec::Support::Spec.setup_simplecov do - minimum_coverage 93 + minimum_coverage 90 end require 'yaml' From d02dd01567e0ca3a250437117af34cebed922f56 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sat, 12 Dec 2020 16:14:43 +0300 Subject: [PATCH 25/26] Pin ffi due to their usage of Gem in 1.13.0/1.13.1 --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 2b76ea42b..1423ace45 100644 --- a/Gemfile +++ b/Gemfile @@ -18,7 +18,7 @@ else gem 'diff-lcs', '~> 1.4', '>= 1.4.3' end -gem 'ffi', '> 1.9.24' # prevent Github security vulnerability warning +gem 'ffi', '~> 1.12.0' gem 'yard', '~> 0.9.24', :require => false From bae42c772884ca42d7b24ef004f15683fa729f09 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Tue, 15 Dec 2020 01:37:20 +0300 Subject: [PATCH 26/26] Update lib/rspec/mocks/method_double.rb Co-authored-by: Jon Rowe --- lib/rspec/mocks/method_double.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/rspec/mocks/method_double.rb b/lib/rspec/mocks/method_double.rb index a8a3b9eeb..c7f758b61 100644 --- a/lib/rspec/mocks/method_double.rb +++ b/lib/rspec/mocks/method_double.rb @@ -259,7 +259,12 @@ def remove_method_from_definition_target # # Note: we could avoid rescuing this by checking # `definition_target.instance_method(@method_name).owner == definition_target`, - # but this error is extremely rare, so we'd rather rescue this exception. + # saving us from the cost of the expensive exception, but this error is + # extremely rare so we'd rather avoid the cost of that check for every + # method double, and risk the rare situation where this exception will + # get raised. This was originally discovered in the core library of older + # unsupported Rubies, (< 2.0) but could happen in code under test + # during meta-programming. RSpec.warn_with( "WARNING: RSpec could not fully restore #{@object.inspect}." \ "#{@method_name}, possibly because the method has been redefined " \