From f72dfbc55774905578086eabde7c6fa90f9aca47 Mon Sep 17 00:00:00 2001 From: Gene Hsu Date: Thu, 18 Nov 2021 17:49:55 +0000 Subject: [PATCH 1/2] Cache match results for compound failures When a compound matcher fails, the failure message checks both sides of the match to determine how to display the failure match. When multiple compound matchers are chained, that means the match for each side of the compound matcher may be called repeatedly for exponential growth. To address this behavior, we can cache the match result for each side of the compound operator each time a match is executed. We need to reset the cache each time the match is executed because the matcher may be re-used for a different expectation. --- lib/rspec/matchers/built_in/compound.rb | 8 +++- spec/rspec/matchers/built_in/compound_spec.rb | 38 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/lib/rspec/matchers/built_in/compound.rb b/lib/rspec/matchers/built_in/compound.rb index 56f27b1a6..0c07d8ba2 100644 --- a/lib/rspec/matchers/built_in/compound.rb +++ b/lib/rspec/matchers/built_in/compound.rb @@ -75,6 +75,8 @@ def initialize_copy(other) end def match(_expected, actual) + @matcher_1_matches = nil + @matcher_2_matches = nil evaluator_klass = if supports_block_expectations? && Proc === actual NestedEvaluator else @@ -97,11 +99,13 @@ def compound_failure_message end def matcher_1_matches? - evaluator.matcher_matches?(matcher_1) + return @matcher_1_matches unless @matcher_1_matches.nil? + @matcher_1_matches = evaluator.matcher_matches?(matcher_1) end def matcher_2_matches? - evaluator.matcher_matches?(matcher_2) + return @matcher_2_matches unless @matcher_2_matches.nil? + @matcher_2_matches = evaluator.matcher_matches?(matcher_2) end def matcher_supports_block_expectations?(matcher) diff --git a/spec/rspec/matchers/built_in/compound_spec.rb b/spec/rspec/matchers/built_in/compound_spec.rb index 0594b6b0a..874f5ae8d 100644 --- a/spec/rspec/matchers/built_in/compound_spec.rb +++ b/spec/rspec/matchers/built_in/compound_spec.rb @@ -872,6 +872,44 @@ def expect_block | EOS end + + context "with long chains of compound matchers" do + let(:failing_matcher) { include(not_expected) } + let(:passing_matcher) { include(expected) } + let(:expected) { actual } + let(:not_expected) { 3 } + let(:actual) { 4 } + + context "with a failing first matcher" do + it "generates a failure description quickly" do + compound = failing_matcher + 15.times { compound = compound.and(passing_matcher) } + expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + end + end + + context "with a failing last matcher" do + it "generates a failure description quickly" do + compound = failing_matcher + 15.times { compound = passing_matcher.and(compound) } + expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + end + end + + context "with all failing matchers" do + it "generates a failure description quickly with and" do + compound = failing_matcher + 15.times { compound = compound.and(failing_matcher) } + expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + end + + it "generates a failure description quickly with or" do + compound = failing_matcher + 15.times { compound = compound.or(failing_matcher) } + expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + end + end + end end describe "expect(...).not_to matcher.or(other_matcher)" do From 6359530cbbb8b0a8bca5ab6e4c8c58af49d505a3 Mon Sep 17 00:00:00 2001 From: Gene Hsu Date: Thu, 18 Nov 2021 21:20:13 +0000 Subject: [PATCH 2/2] Refactor `timeout_if_not_debugging` Refactor `timeout_if_not_debugging` into `spec_helper` and use it in the compound matcher spec to limit the run time when testing long chains of compound matchers. --- spec/rspec/matchers/built_in/compound_spec.rb | 32 ++++++++++++------- .../matchers/built_in/contain_exactly_spec.rb | 8 ----- spec/spec_helper.rb | 8 +++++ 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/spec/rspec/matchers/built_in/compound_spec.rb b/spec/rspec/matchers/built_in/compound_spec.rb index 874f5ae8d..a562b7eeb 100644 --- a/spec/rspec/matchers/built_in/compound_spec.rb +++ b/spec/rspec/matchers/built_in/compound_spec.rb @@ -882,31 +882,39 @@ def expect_block context "with a failing first matcher" do it "generates a failure description quickly" do - compound = failing_matcher - 15.times { compound = compound.and(passing_matcher) } - expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + timeout_if_not_debugging(0.2) do + compound = failing_matcher + 15.times { compound = compound.and(passing_matcher) } + expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + end end end context "with a failing last matcher" do it "generates a failure description quickly" do - compound = failing_matcher - 15.times { compound = passing_matcher.and(compound) } - expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + timeout_if_not_debugging(0.2) do + compound = failing_matcher + 15.times { compound = passing_matcher.and(compound) } + expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + end end end context "with all failing matchers" do it "generates a failure description quickly with and" do - compound = failing_matcher - 15.times { compound = compound.and(failing_matcher) } - expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + timeout_if_not_debugging(0.2) do + compound = failing_matcher + 15.times { compound = compound.and(failing_matcher) } + expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + end end it "generates a failure description quickly with or" do - compound = failing_matcher - 15.times { compound = compound.or(failing_matcher) } - expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + timeout_if_not_debugging(0.2) do + compound = failing_matcher + 15.times { compound = compound.or(failing_matcher) } + expect { expect([actual]).to compound }.to fail_including("expected [#{actual}] to include #{not_expected}") + end end 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..5d48c77f0 100644 --- a/spec/rspec/matchers/built_in/contain_exactly_spec.rb +++ b/spec/rspec/matchers/built_in/contain_exactly_spec.rb @@ -184,14 +184,6 @@ 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 } - 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 ] diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 595fafa2c..bb5a59486 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,6 +25,14 @@ def dedent(string) string.gsub(/^\s+\|/, '').chomp end + 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 + # We have to use Hash#inspect in examples that have multi-entry # hashes because the #inspect output on 1.8.7 is non-deterministic # due to the fact that hashes are not ordered. So we can't simply