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

Speed up the ContainExactly matcher #1333

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
39 changes: 34 additions & 5 deletions lib/rspec/matchers/built_in/contain_exactly.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,46 @@ def best_solution
@best_solution ||= pairings_maximizer.find_best_solution
end

def pairings_maximizer
def pairings_maximizer # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
@pairings_maximizer ||= begin
expected_matches = Hash[Array.new(expected.size) { |i| [i, []] }]
actual_matches = Hash[Array.new(actual.size) { |i| [i, []] }]

expected.each_with_index do |e, ei|
actual.each_with_index do |a, ai|
# Set the reciprocal pairings for matching elements
value_buckets = Hash.new { |hash, key| hash[key] = [] }
expected.each_with_index { |v, i| value_buckets[[v, :expected]] << i }
actual.each_with_index { |v, i| value_buckets[[v, :actual]] << i }
value_buckets.each do |key, indexes|
value, source = key
next unless source == :expected
next unless value_buckets.key? [value, :actual]

actual_indexes = value_buckets[[value, :actual]]
indexes.zip(actual_indexes).each do |expected_index, actual_index|
break unless actual_index

expected_matches[expected_index] << actual_index
actual_matches[actual_index] << expected_index
end
end

# Set the remaining elements to be paired
filtered_expected = []
filtered_actual = []
expected.each_with_index do |e, expected_index|
filtered_expected << [e, expected_index] if expected_matches[expected_index].empty?
end
actual.each_with_index do |a, actual_index|
filtered_actual << [a, actual_index] if actual_matches[actual_index].empty?
end

# Set the pairing combinations of the remaining elements
filtered_expected.each do |e, expected_index|
filtered_actual.each do |a, actual_index|
next unless values_match?(e, a)

expected_matches[ei] << ai
actual_matches[ai] << ei
expected_matches[expected_index] << actual_index
actual_matches[actual_index] << expected_index
end
end

Expand Down
78 changes: 65 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,74 @@ 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 }
context "speed checks" 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]
shared_examples "succeeds fast" do
pirj marked this conversation as resolved.
Show resolved Hide resolved
it do
timeout_if_not_debugging(max_runtime) do
subject
pirj marked this conversation as resolved.
Show resolved Hide resolved
end
end
end

expect {
expect(actual).to contain_exactly(*expected)
}.to fail_including("the missing elements were: [0, 1, 4, 4]")
shared_examples "fails fast" do |failure_msg|
pirj marked this conversation as resolved.
Show resolved Hide resolved
it do
timeout_if_not_debugging(max_runtime) do
expect {
subject
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
subject
expect_actual_to_contain_exactly_expected

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subject is a substitute for expect_actual_to_contain_exactly_expected or expect_actual_not_to_contain_exactly_expected depending on the including example group.

}.to fail_with(/#{Regexp.quote(failure_msg)}/)
end
end
end

let(:max_runtime) { 0.5 }
let(:actual) { Array.new(1000) { rand(10) } }

context "with a positive expectation" do
subject { expect(actual).to contain_exactly(*expected) }
pirj marked this conversation as resolved.
Show resolved Hide resolved

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

it "matches" do
subject
pirj marked this conversation as resolved.
Show resolved Hide resolved
end

include_examples "succeeds fast"
pirj marked this conversation as resolved.
Show resolved Hide resolved
end

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

include_examples "fails fast", "expected collection contained"
pirj marked this conversation as resolved.
Show resolved Hide resolved
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(1000) { rand(10) } }

it "does not match" do
subject
end

include_examples "succeeds fast"
pirj marked this conversation as resolved.
Show resolved Hide resolved
end

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

include_examples "fails fast", "not to contain exactly"
pirj marked this conversation as resolved.
Show resolved Hide resolved
end
end
end

Expand Down