From 7972abf123c5b5f9919a1ef7346453f4edaf7571 Mon Sep 17 00:00:00 2001 From: Benjamin Clayman Date: Fri, 29 Oct 2021 13:00:35 -0400 Subject: [PATCH] Speed up the ContainExactly matcher Speed up the ContainExactly matcher by pre-emptively matching up corresponding elements in the expected and actual arrays. This addresses #1006, #1161. This PR is a collaboration between me and @genehsu based on a couple of our earlier PRs and discussion that resulted: 1) https://github.com/rspec/rspec-expectations/pull/1325 2) https://github.com/rspec/rspec-expectations/pull/1328 Co-authored-by: Gene Hsu (@genehsu) --- .../matchers/built_in/contain_exactly.rb | 39 ++++++++-- .../matchers/built_in/contain_exactly_spec.rb | 78 +++++++++++++++---- 2 files changed, 99 insertions(+), 18 deletions(-) diff --git a/lib/rspec/matchers/built_in/contain_exactly.rb b/lib/rspec/matchers/built_in/contain_exactly.rb index 1bf7f9831..f3cb69518 100644 --- a/lib/rspec/matchers/built_in/contain_exactly.rb +++ b/lib/rspec/matchers/built_in/contain_exactly.rb @@ -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 diff --git a/spec/rspec/matchers/built_in/contain_exactly_spec.rb b/spec/rspec/matchers/built_in/contain_exactly_spec.rb index ac1996ae8..8629aff64 100644 --- a/spec/rspec/matchers/built_in/contain_exactly_spec.rb +++ b/spec/rspec/matchers/built_in/contain_exactly_spec.rb @@ -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 + it do + timeout_if_not_debugging(max_runtime) do + subject + 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| + it do + timeout_if_not_debugging(max_runtime) do + expect { + subject + }.to fail_with(/#{Regexp.quote(failure_msg)}/) + end + end + end + + let(:max_runtime) { 1 } + 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