diff --git a/Changelog.md b/Changelog.md index 6412a5794a..a0898cf0bd 100644 --- a/Changelog.md +++ b/Changelog.md @@ -15,6 +15,8 @@ Breaking Changes: an unsupported value. (Phil Pirozhkov, #2849) * Remove deprecated access to an example group's metadata through the example. (Phil Pirozhkov, #2851) +* Remove deprecated Hash-like behavior from example + execution result. (Phil Pirozhkov, #2862) Enhancements: diff --git a/lib/rspec/core/example.rb b/lib/rspec/core/example.rb index 5b6464a75d..3f9aa2aa83 100644 --- a/lib/rspec/core/example.rb +++ b/lib/rspec/core/example.rb @@ -551,10 +551,7 @@ def location_description end # Represents the result of executing an example. - # Behaves like a hash for backwards compatibility. class ExecutionResult - include HashImitatable - # @return [Symbol] `:passed`, `:failed` or `:pending`. attr_accessor :status @@ -616,32 +613,6 @@ def calculate_run_time(finished_at) self.finished_at = finished_at self.run_time = (finished_at - started_at).to_f end - - # For backwards compatibility we present `status` as a string - # when presenting the legacy hash interface. - def hash_for_delegation - super.tap do |hash| - hash[:status] &&= status.to_s - end - end - - def set_value(name, value) - value &&= value.to_sym if name == :status - super(name, value) - end - - def get_value(name) - if name == :status - status.to_s if status - else - super - end - end - - def issue_deprecation(_method_name, *_args) - RSpec.deprecate("Treating `metadata[:execution_result]` as a hash", - :replacement => "the attributes methods to access the data") - end end end diff --git a/lib/rspec/core/metadata.rb b/lib/rspec/core/metadata.rb index 7ff8f6c561..264c475e48 100644 --- a/lib/rspec/core/metadata.rb +++ b/lib/rspec/core/metadata.rb @@ -290,107 +290,5 @@ def full_description :shared_group_inclusion_backtrace ] end - - # Mixin that makes the including class imitate a hash for backwards - # compatibility. The including class should use `attr_accessor` to - # declare attributes. - # @private - module HashImitatable - def self.included(klass) - klass.extend ClassMethods - end - - def to_h - hash = extra_hash_attributes.dup - - self.class.hash_attribute_names.each do |name| - hash[name] = __send__(name) - end - - hash - end - - (Hash.public_instance_methods - Object.public_instance_methods).each do |method_name| - next if [:[], :[]=, :to_h].include?(method_name.to_sym) - - define_method(method_name) do |*args, &block| - issue_deprecation(method_name, *args) - - hash = hash_for_delegation - self.class.hash_attribute_names.each do |name| - hash.delete(name) unless instance_variable_defined?(:"@#{name}") - end - - hash.__send__(method_name, *args, &block).tap do - # apply mutations back to the object - hash.each do |name, value| - if directly_supports_attribute?(name) - set_value(name, value) - else - extra_hash_attributes[name] = value - end - end - end - end - end - - def [](key) - issue_deprecation(:[], key) - - if directly_supports_attribute?(key) - get_value(key) - else - extra_hash_attributes[key] - end - end - - def []=(key, value) - issue_deprecation(:[]=, key, value) - - if directly_supports_attribute?(key) - set_value(key, value) - else - extra_hash_attributes[key] = value - end - end - - private - - def extra_hash_attributes - @extra_hash_attributes ||= {} - end - - def directly_supports_attribute?(name) - self.class.hash_attribute_names.include?(name) - end - - def get_value(name) - __send__(name) - end - - def set_value(name, value) - __send__(:"#{name}=", value) - end - - def hash_for_delegation - to_h - end - - def issue_deprecation(_method_name, *_args) - # no-op by default: subclasses can override - end - - # @private - module ClassMethods - def hash_attribute_names - @hash_attribute_names ||= [] - end - - def attr_accessor(*names) - hash_attribute_names.concat(names) - super - end - end - end end end diff --git a/spec/rspec/core/example_execution_result_spec.rb b/spec/rspec/core/example_execution_result_spec.rb index a76dff9fca..9611373169 100644 --- a/spec/rspec/core/example_execution_result_spec.rb +++ b/spec/rspec/core/example_execution_result_spec.rb @@ -2,19 +2,8 @@ module RSpec module Core class Example RSpec.describe ExecutionResult do - it "supports ruby 2.1's `to_h` protocol" do - er = ExecutionResult.new - er.run_time = 17 - er.pending_message = "just because" - - expect(er.to_h).to include( - :run_time => 17, - :pending_message => "just because" - ) - end - - it 'includes all defined attributes in the `to_h` hash even if not set' do - expect(ExecutionResult.new.to_h).to include( + it 'has `status` and `pending_message` attributes' do + expect(ExecutionResult.new).to have_attributes( :status => nil, :pending_message => nil ) @@ -25,111 +14,12 @@ class Example expect { er.pending_fixed = true }.to change(er, :pending_fixed?).from(false).to(true) end - describe "backwards compatibility" do - it 'supports indexed access like a hash' do - er = ExecutionResult.new - er.started_at = (started_at = ::Time.utc(2014, 3, 1, 12, 30)) - expect_deprecation_with_call_site(__FILE__, __LINE__ + 1, /execution_result/) - expect(er[:started_at]).to eq(started_at) - end - - it 'supports indexed updates like a hash' do - er = ExecutionResult.new - expect_deprecation_with_call_site(__FILE__, __LINE__ + 1, /execution_result/) - er[:started_at] = (started_at = ::Time.utc(2014, 3, 1, 12, 30)) - expect(er.started_at).to eq(started_at) - end - - it 'can get and set user defined attributes like with a hash' do - er = ExecutionResult.new - allow_deprecation - expect { er[:foo] = 3 }.to change { er[:foo] }.from(nil).to(3) - expect(er.to_h).to include(:foo => 3) - end - - it 'supports `update` like a hash' do - er = ExecutionResult.new - expect_deprecation_with_call_site(__FILE__, __LINE__ + 1, /execution_result/) - er.update(:pending_message => "some message", :exception => ArgumentError.new) - expect(er.pending_message).to eq("some message") - expect(er.exception).to be_a(ArgumentError) - end - - it 'can set undefined attribute keys through any hash mutation method' do - allow_deprecation - er = ExecutionResult.new - er.update(:pending_message => "msg", :foo => 3) - expect(er.to_h).to include(:pending_message => "msg", :foo => 3) - end - - it 'supports `merge` like a hash' do - er = ExecutionResult.new - er.exception = ArgumentError.new - er.pending_message = "just because" - - expect_deprecation_with_call_site(__FILE__, __LINE__ + 1, /execution_result/) - merged = er.merge(:exception => NotImplementedError.new, :foo => 3) - - expect(merged).to include( - :exception => an_instance_of(NotImplementedError), - :pending_message => "just because", - :foo => 3 - ) - - expect(er.exception).to be_an(ArgumentError) - end - - it 'supports blocks for hash methods that support one' do - er = ExecutionResult.new - expect_deprecation_with_call_site(__FILE__, __LINE__ + 1, /execution_result/) - expect(er.fetch(:foo) { 3 }).to eq(3) - end - - specify '#fetch treats unset properties the same as a hash does' do - allow_deprecation - er = ExecutionResult.new - expect { er.fetch(:pending_message) }.to raise_error(KeyError) - er.pending_message = "some msg" - expect(er.fetch(:pending_message)).to eq("some msg") - end - - describe "status" do - it 'returns a string when accessed like a hash' do - er = ExecutionResult.new - expect(er[:status]).to eq(nil) - er.status = :failed - expect(er[:status]).to eq("failed") - end - - it "sets the status to a symbol when assigned as a string via the hash interface" do - er = ExecutionResult.new - er[:status] = "failed" - expect(er.status).to eq(:failed) - er[:status] = nil - expect(er.status).to eq(nil) - end - - it "is presented as a string when included in returned hashes" do - er = ExecutionResult.new - er.status = :failed - expect(er.merge(:foo => 3)).to include(:status => "failed", :foo => 3) - - er.status = nil - expect(er.merge(:foo => 3)).to include(:status => nil, :foo => 3) - end - - it "is updated to a symbol when updated as a string via `update`" do - er = ExecutionResult.new - er.update(:status => "passed") - expect(er.status).to eq(:passed) - end + it 'does not support `to_h`' do + expect(ExecutionResult.new.respond_to?(:to_h)).to be false + end - it 'is presented as a symbol in `to_h`' do - er = ExecutionResult.new - er.status = :failed - expect(er.to_h).to include(:status => :failed) - end - end + it 'does not behave like a hash' do + expect(ExecutionResult.new.respond_to?(:[])).to be false end end end diff --git a/spec/rspec/core/example_group_spec.rb b/spec/rspec/core/example_group_spec.rb index c4f92076e6..6881e6b242 100644 --- a/spec/rspec/core/example_group_spec.rb +++ b/spec/rspec/core/example_group_spec.rb @@ -1138,8 +1138,8 @@ def extract_execution_results(group) end group.run - expect(extract_execution_results(group).map(&:to_h)).to match([ - a_hash_including( + expect(extract_execution_results(group)).to match([ + have_attributes( :status => :pending, :pending_message => "Temporarily skipped with #{method_name}" ) @@ -1190,12 +1190,12 @@ def extract_execution_results(group) end group.run - expect(extract_execution_results(group).map(&:to_h)).to match([ - a_hash_including( + expect(extract_execution_results(group)).to match([ + have_attributes( :status => :failed, :pending_message => "No reason given" ), - a_hash_including( + have_attributes( :status => :pending, :pending_message => "unimplemented" ) diff --git a/spec/rspec/core/metadata_spec.rb b/spec/rspec/core/metadata_spec.rb index 82a7e192bf..5b6f3be866 100644 --- a/spec/rspec/core/metadata_spec.rb +++ b/spec/rspec/core/metadata_spec.rb @@ -102,7 +102,9 @@ def metadata_for(*args) end it "creates an empty execution result" do - expect(example_metadata[:execution_result].to_h.reject { |_, v| v.nil? } ).to eq({}) + expect(example_metadata[:execution_result]) + .to be_an(Example::ExecutionResult) + .and have_attributes(:status => nil) end it "extracts file path from caller" do