Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ContainExactly matcher speed when elements obey transitivity #1325

Closed
Show file tree
Hide file tree
Changes from 4 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
55 changes: 53 additions & 2 deletions lib/rspec/matchers/built_in/contain_exactly.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def description
def generate_failure_message
message = expected_collection_line
message += actual_collection_line
@extra_items, @missing_items = fast_calculate_extra_missing if @transitive
message += missing_elements_line unless missing_items.empty?
message += extra_elements_line unless extra_items.empty?
message
Expand Down Expand Up @@ -72,15 +73,23 @@ def message_line(prefix, collection, surface_descriptions=false)

def match(_expected, _actual)
return false unless convert_actual_to_an_array
match_when_sorted? || (extra_items.empty? && missing_items.empty?)

matched_when_sorted = match_when_sorted?

return true if matched_when_sorted
return matched_when_sorted if @transitive
pirj marked this conversation as resolved.
Show resolved Hide resolved

(extra_items.empty? && missing_items.empty?)
end

# This cannot always work (e.g. when dealing with unsortable items,
# or matchers as expected items), but it's practically free compared to
# the slowness of the full matching algorithm, and in common cases this
# works, so it's worth a try.
def match_when_sorted?
values_match?(safe_sort(expected), safe_sort(actual))
@transitive = true # Be optimistic. Let `safe_sort` reveal non-transitivity.
@sorted_expected, @sorted_actual = safe_sort(expected), safe_sort(actual)
values_match?(@sorted_expected, @sorted_actual)
end

def convert_actual_to_an_array
Expand All @@ -96,6 +105,7 @@ def convert_actual_to_an_array
def safe_sort(array)
array.sort
rescue Support::AllExceptionsExceptOnesWeMustNotRescue
@transitive = false
array
end

Expand Down Expand Up @@ -124,6 +134,47 @@ def extra_items
end
end

# We use this to determine extra and missing items between expected
# and actual arrays. This runs in O(n) time which is a big improvement
# over the O(n!) work incurred by PairingsMaximizer to evaluate all possible
# matchings between arrays
# rubocop:disable MethodLength
# rubocop:disable Metrics/AbcSize
def fast_calculate_extra_missing
pirj marked this conversation as resolved.
Show resolved Hide resolved
extra, missing = [], []
i, j = 0, 0

# Use 2-pointer approach to find elements in sorted_actual
# that aren't in sorted_expected and vice versa
while i < @sorted_actual.size && j < @sorted_expected.size
current_actual, current_expected = @sorted_actual[i], @sorted_expected[j]

if current_actual < current_expected
extra << current_actual
i += 1
elsif current_actual > current_expected
missing << current_expected
j += 1
else
i += 1
j += 1
end
end

while i < @sorted_actual.size
extra << @sorted_actual[i]
pirj marked this conversation as resolved.
Show resolved Hide resolved
i += 1
end
while j < @sorted_expected.size
missing << @sorted_expected[j]
j += 1
end

[extra, missing]
end
# rubocop:enable MethodLength
# rubocop:enable Metrics/AbcSize

def best_solution
@best_solution ||= pairings_maximizer.find_best_solution
end
Expand Down
71 changes: 71 additions & 0 deletions spec/rspec/matchers/built_in/contain_exactly_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,77 @@ def array.send; :sent; end
end

RSpec.describe "using contain_exactly with expect" do
# users have reported using contains_exactly with 50 elements
# never finishing!
context "with transitive enabled" do
require 'benchmark'

context "with actual and expected containing sortable elements" do
shared_examples "runs very fast" do
pirj marked this conversation as resolved.
Show resolved Hide resolved
it do
time = Benchmark.realtime do
subject
end
# this is in seconds
expect(time).to be < 0.3
end
end

let(:a) { Array.new(10_000) { rand(10) } }

context "with a positive expectation" do
subject { expect(a).to contain_exactly(*b) }

context "that is valid" do
let(:b) { a.shuffle }

it "matches" do
subject
end

include_examples "runs very fast"
end

context "that is not valid" do
let(:b) { Array.new(10_000) { rand(10) } }

it "fails quickly" do
time = Benchmark.realtime do
expect { subject }.to fail_with(/expected collection contained/)
end
expect(time).to be < 1
end

end
end

context "with a negative expectation" do
subject { expect(a).not_to contain_exactly(*b) }

context "that is valid" do
let(:b) { Array.new(10_000) { rand(10) } }

it "does not match" do
subject
end

include_examples "runs very fast"
end

context "that is not valid" do
let(:b) { a.shuffle }

it "fails quickly" do
time = Benchmark.realtime do
expect { expect(a).not_to contain_exactly(*b) }.to fail_with(/not to contain exactly/)
end
expect(time).to be < 1
end
end
end
end
end

it "passes a valid positive expectation" do
expect([1, 2]).to contain_exactly(2, 1)
end
Expand Down