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

Commit

Permalink
Improve change mather detection of objects that have changed. (#1132)
Browse files Browse the repository at this point in the history
* Add instance variables to changes detected

* Add benchmark for large objects
  • Loading branch information
JonRowe authored Sep 10, 2019
1 parent 5e0cf19 commit 443212a
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 1 deletion.
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

0 comments on commit 443212a

Please sign in to comment.