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 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
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
94 changes: 81 additions & 13 deletions spec/rspec/matchers/built_in/contain_exactly_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,22 +184,90 @@ def array.send; :sent; end
MESSAGE
end

def timeout_if_not_debugging(time)
in_sub_process_if_possible do
require 'timeout'
return yield if defined?(::Debugger)
Timeout.timeout(time) { yield }
# users have reported using contains_exactly with 50 elements
# never finishing!
context 'with comparable elements' do
def timeout_if_not_debugging(time)
in_sub_process_if_possible do
require 'timeout'
return yield if defined?(::Debugger)
Timeout.timeout(time) { yield }
end
end
end

it 'fails a match of 11 items with duplicates in a reasonable amount of time' do
timeout_if_not_debugging(0.1) do
expected = [0, 1, 1, 3, 3, 3, 4, 4, 8, 8, 9 ]
actual = [ 1, 2, 3, 3, 3, 3, 7, 8, 8, 9, 9]
it 'fails a match of 11 items with duplicates in a reasonable amount of time' do
timeout_if_not_debugging(0.1) do
expected = [0, 1, 1, 3, 3, 3, 4, 4, 8, 8, 9 ]
actual = [ 1, 2, 3, 3, 3, 3, 7, 8, 8, 9, 9]

expect {
expect(actual).to contain_exactly(*expected)
}.to fail_including("the missing elements were: [0, 1, 4, 4]")
expect {
expect(actual).to contain_exactly(*expected)
}.to fail_including("the missing elements were: [0, 1, 4, 4]")
end
end

context "with actual and expected containing sortable elements" do
let(:max_runtime) { 1 }

shared_examples "succeeds fast" do
it do
timeout_if_not_debugging(max_runtime) do
subject
end
end
end

shared_examples "fails fast" do |failure_msg|
it do
timeout_if_not_debugging(max_runtime) do
expect {
subject
}.to fail_with(/#{Regexp.quote(failure_msg)}/)
end
end
end

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

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

context "that is valid" do
let(:expected) { actual.shuffle }

it "matches" do
subject
end

include_examples "succeeds fast"
end

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

include_examples "fails fast", "expected collection contained"
end
end

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

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

it "does not match" do
subject
end

include_examples "succeeds fast"
end

context "that is not valid" do
let(:expected) { actual.shuffle }

include_examples "fails fast", "not to contain exactly"
end
end
end
end

Expand Down