Skip to content

Commit

Permalink
Remove deprecated Hash-like behavior from example execution result
Browse files Browse the repository at this point in the history
  • Loading branch information
@pirj
pirj committed Feb 10, 2021
1 parent d7603e5 commit 6da0e66
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 254 deletions.
2 changes: 2 additions & 0 deletions Changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Breaking Changes:
an unsupported value. (Phil Pirozhkov, #2849)
* Remove deprecated exposition of example group's metadata
in example's metadata `:example_group` subhash. (Phil Pirozhkov, #2851)
* Remove deprecated Hash-like behavior from example
execution result. (Phil Pirozhkov, #2862)

Enhancements:

Expand Down
29 changes: 0 additions & 29 deletions lib/rspec/core/example.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
102 changes: 0 additions & 102 deletions lib/rspec/core/metadata.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
126 changes: 9 additions & 117 deletions spec/rspec/core/example_execution_result_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand All @@ -25,111 +14,14 @@ 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.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
er = ExecutionResult.new
er.started_at = (started_at = ::Time.utc(2014, 3, 1, 12, 30))
expect(er.respond_to?(:[])).to be false
end
end
end
Expand Down
10 changes: 5 additions & 5 deletions spec/rspec/core/example_group_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
)
Expand Down Expand Up @@ -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"
)
Expand Down
4 changes: 3 additions & 1 deletion spec/rspec/core/metadata_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 6da0e66

Please sign in to comment.