Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Improve change matcher detection of objects that have changed. #1132

Merged
merged 2 commits into from
Sep 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions benchmarks/hashing_large_objects.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
require 'benchmark/ips'

OBJECT_VARIABLES = Object.instance_method(:instance_variables)
OBJECT_VARIABLE_GET = Object.instance_method(:instance_variable_get)

def calculate_attribute_hashes(object)
OBJECT_VARIABLES.bind(object).call.map { |name| OBJECT_VARIABLE_GET.bind(object).call(name).hash }
end

[10, 100, 1000, 10000].each do |object_size|
klass = Class.new do
define_method(:initialize) do
object_size.times do |i|
instance_variable_set(:"@my_var_#{i}", 'a' * (rand * 100))
end
end
end
instance = klass.new

Benchmark.ips do |ips|
ips.report("Get hashes for object of size: #{instance.instance_variables.size}") do
calculate_attribute_hashes(instance)
end
end
end

# Warming up --------------------------------------
# Get hashes for object of size: 10
# 14.412k i/100ms
# Calculating -------------------------------------
# Get hashes for object of size: 10
# 158.585k (± 5.2%) i/s - 792.660k in 5.012663s
# Warming up --------------------------------------
# Get hashes for object of size: 100
# 1.664k i/100ms
# Calculating -------------------------------------
# Get hashes for object of size: 100
# 17.041k (± 9.0%) i/s - 84.864k in 5.036735s
# Warming up --------------------------------------
# Get hashes for object of size: 1000
# 173.000 i/100ms
# Calculating -------------------------------------
# Get hashes for object of size: 1000
# 1.808k (± 4.6%) i/s - 9.169k in 5.082355s
# Warming up --------------------------------------
# Get hashes for object of size: 10000
# 16.000 i/100ms
# Calculating -------------------------------------
# Get hashes for object of size: 10000
# 176.298 (± 3.4%) i/s - 896.000 in 5.089070s
15 changes: 14 additions & 1 deletion lib/rspec/matchers/built_in/change.rb
Original file line number Diff line number Diff line change
Expand Up @@ -367,13 +367,16 @@ def value_representation
def perform_change(event_proc)
@actual_before = evaluate_value_proc
@before_hash = @actual_before.hash
@before_attribute_hashes = calculate_attribute_hashes(@actual_before)

yield @actual_before if block_given?

return false unless Proc === event_proc
event_proc.call

@actual_after = evaluate_value_proc
@actual_hash = @actual_after.hash
@after_attribute_hashes = calculate_attribute_hashes(@actual_after)
true
end

Expand All @@ -390,7 +393,9 @@ def changed?
# possible for two values to be unequal (and of different classes)
# but to return the same hash value. Also, some objects may change
# their hash after being compared with `==`/`!=`.
@actual_before != @actual_after || @before_hash != @actual_hash
@actual_before != @actual_after ||
@before_hash != @actual_hash ||
@before_attribute_hashes != @after_attribute_hashes
end

def actual_delta
Expand All @@ -399,6 +404,14 @@ def actual_delta

private

# methods are stashed to ensure we get access to variables on modified classes / BasicObject
OBJECT_VARIABLES = Object.instance_method(:instance_variables)
OBJECT_VARIABLE_GET = Object.instance_method(:instance_variable_get)

def calculate_attribute_hashes(object)
OBJECT_VARIABLES.bind(object).call.map { |name| OBJECT_VARIABLE_GET.bind(object).call(name).hash }
end

def evaluate_value_proc
@value_proc ? @value_proc.call : @receiver.__send__(@message)
end
Expand Down
4 changes: 4 additions & 0 deletions spec/rspec/matchers/built_in/change_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,10 @@ def self.to_s
expect { @instance.some_value = 6 }.to change { @instance.some_value }
end

it "passes when actual has a value modified" do
expect { @instance.some_value = 6 }.to change { @instance }
end

it "fails when actual is not modified by the block" do
expect do
expect {}.to change { @instance.some_value }
Expand Down