From 47d409f7d08da50c377d6353a45e7dbc3e2249f1 Mon Sep 17 00:00:00 2001 From: Benjamin Clayman Date: Mon, 4 Oct 2021 15:16:22 -0400 Subject: [PATCH] Fix Include Matcher For Ranges This is issue #1191. Previously, a few parts of the Include matcher assumed all Ranges were iterable. This caused it to raise errors like: TypeError: Can't iterate from [Float|Time] This happens because Ranges require that their beginning element implement succ. Float doesn't which causes the error. Time is different because it does implement succ but a) it's deprecated as of Ruby 1.9.2 and b) some Ruby implementations raise an exception when trying to iterate through a range of Time objects. This PR does a few things: 1) Fixes the Include matcher to handle Ranges that don't support iteration, while continuing to support Ranges that do 2) Adds specs for both types of Ranges in 1). There weren't any for the Include matcher used with Ranges. --- lib/rspec/matchers/built_in/include.rb | 16 + spec/rspec/matchers/built_in/include_spec.rb | 560 ++++++++++++++++++- 2 files changed, 575 insertions(+), 1 deletion(-) diff --git a/lib/rspec/matchers/built_in/include.rb b/lib/rspec/matchers/built_in/include.rb index 5a0d697f0..3df3b0537 100644 --- a/lib/rspec/matchers/built_in/include.rb +++ b/lib/rspec/matchers/built_in/include.rb @@ -163,6 +163,9 @@ def actual_collection_includes?(expected_item) # String lacks an `any?` method... return false unless actual.respond_to?(:any?) + # Some objects don't support iteration (e.g. Float, Time, etc.) + return false if non_iterable_range?(actual) + actual.any? { |value| values_match?(expected_item, value) } end @@ -172,10 +175,23 @@ def count_enumerable(expected_item) end else def count_enumerable(expected_item) + return actual.include?(expected_item) ? 1 : 0 if non_iterable_range?(actual) actual.count { |value| values_match?(expected_item, value) } end end + # Determines if actual is a range and contains elements that do not support iteration. + # The usual requirement for a Range supporting iteration is that its beginning element + # implements `succ`. Time is different; it implements `succ` but that method is + # deprecated as of 1.9.2. Attempting to iterate through a Range of Times raises a + # TypeError on many common Ruby implementations. We treat Ranges of Time objects as + # non-iterable to ensure safety. + def non_iterable_range?(actual) + # leave old behavior untouched, but fix for modern rubies + return false unless RUBY_VERSION >= "2.1.9" + actual.is_a?(Range) && (!actual.min.respond_to?(:succ) || actual.min.is_a?(Time)) + end + def count_inclusions @divergent_items = expected case actual diff --git a/spec/rspec/matchers/built_in/include_spec.rb b/spec/rspec/matchers/built_in/include_spec.rb index e9649f66c..7997d6102 100644 --- a/spec/rspec/matchers/built_in/include_spec.rb +++ b/spec/rspec/matchers/built_in/include_spec.rb @@ -111,6 +111,20 @@ def hash.send; :sent; end end end + shared_context "time range" do + let(:fmt) { RSpec::Support::ObjectFormatter } + let(:now) { Time.now.utc } + let(:start) { now - 20 } + let(:finish) { now - 10 } + let(:range) { (start..finish) } + end + + shared_context "only runs for modern rubies" do + around(:example) do |example| + example.run if RUBY_VERSION >= "2.1.9" + end + end + describe "expect(...).to include(with_one_arg)" do it_behaves_like "an RSpec value matcher", :valid_value => [1, 2], :invalid_value => [1] do let(:matcher) { include(2) } @@ -148,6 +162,165 @@ def hash.send; :sent; end end end + context "for a range target" do + include_context "only runs for modern rubies" + + context "with elements that support iteration" do + let(:range) { 1..3 } + + it "passes if target includes expected" do + expect(range).to include(2) + end + + it "fails if target does not include expected" do + expect { + expect(range).to include(4) + }.to fail_matching("expected #{range} to include 4") + end + + context "with exact count" do + it "fails if the block yields the wrong number of times" do + expect { + expect(range).to include(2).twice + }.to fail_with("expected #{range} to include 2 twice but it is included once") + end + + it "passes if the block yields the specified number of times" do + expect(range).to include(2).once + end + end + + context "with at least count" do + it "passes if the search term is included at least the number of times" do + expect(range).to include(1).at_least(1).times + expect(range).to include(1).at_least(:once) + end + + it "fails if the search term is included too few times" do + expect { + expect(range).to include(1).at_least(:twice) + }.to fail_with("expected #{range} to include 1 at least twice but it is included once") + end + end + + context "with at most count" do + it "passes if the search term is included at most the number of times" do + expect(range).to include(1).at_most(2).times + expect(range).to include(1).at_most(:twice) + end + end + end + + context 'with elements that do not support iteration' do + context 'with floats' do + let(:range) { 1.0..3.0 } + + it "passes if target includes expected" do + expect(range).to include(2.0) + end + + it "fails if target does not include expected" do + expect { + expect(range).to include(4.0) + }.to fail_matching("expected #{range} to include 4.0") + end + + context "with exact count" do + it "fails if the block yields the wrong number of times" do + expect { + expect(range).to include(2.0).twice + }.to fail_with("expected #{range} to include 2.0 twice but it is included once") + end + + it "passes if the block yields the specified number of times" do + expect(range).to include(2.0).once + end + end + + context "with at least count" do + it "passes if the search term is included at least the number of times" do + expect(range).to include(1.0).at_least(1).times + expect(range).to include(1.0).at_least(:once) + end + + it "fails if the search term is included too few times" do + expect { + expect(range).to include(1.0).at_least(:twice) + }.to fail_with("expected #{range} to include 1.0 at least twice but it is included once") + end + end + + context "with at most count" do + it "passes if the search term is included at most the number of times" do + expect(range).to include(1.0).at_most(2).times + expect(range).to include(1.0).at_most(:twice) + end + end + end + + context "with times" do + include_context "time range" + + it "passes if target includes expected" do + expect(range).to include(start) + end + + it "fails if target does not include expected" do + expect { + expect(range).to include(start-1) + }.to fail_matching("expected #{range.inspect} to include #{fmt.format(start-1)}") + end + + context "with exact count" do + it "fails if the block yields the wrong number of times" do + expect { + expect(range).to include(start).twice + }.to fail_matching("expected #{range.inspect} to include #{fmt.format(start)} twice but it is included once") + end + + it "passes if the block yields the specified number of times" do + expect(range).to include(start).once + end + end + + context "with at least count" do + it "passes if the search term is included at least the number of times" do + expect(range).to include(start).at_least(1).times + expect(range).to include(start).at_least(:once) + end + + it "fails if the search term is included too few times" do + expect { + expect(range).to include(start).at_least(:twice) + }.to fail_matching("expected #{range.inspect} to include #{fmt.format(start)} at least twice but it is included once") + end + end + + context "with at most count" do + it "passes if the search term is included at most the number of times" do + expect(range).to include(start).at_most(2).times + expect(range).to include(start).at_most(:twice) + end + end + end + end + + it "fails when given differing null doubles" do + dbl_1 = double.as_null_object + dbl_2 = double.as_null_object + + expect { + expect(dbl_1..dbl_1).to include(dbl_2) + }.to fail_matching("expected #{dbl_1.inspect}..#{dbl_1.inspect} to include") + end + + it "passes when given the same null double" do + dbl = double.as_null_object + + expect(dbl..dbl).to include(dbl) + end + end + context "for a string target" do it "passes if target includes expected" do expect("abc").to include("a") @@ -329,6 +502,94 @@ class PseudoHash < SimpleDelegator matcher = include("a") expect(matcher.description).to eq("include \"a\"") end + + context "for a range target" do + include_context "only runs for modern rubies" + + context "with elements that support iteration" do + let(:range) { 1..3 } + + it "passes if target includes all items" do + expect(range).to include(1, 2) + end + + it "fails if target does not include one of the items" do + expect { + expect(range).to include(2, 3, 4) + }.to fail_matching("expected #{range} to include 4") + end + + it "fails if target does not include two of the items" do + expect { + expect(range).to include(3, 4, 5) + }.to fail_matching("expected #{range} to include 4 and 5") + end + + it "fails if target does not include many of the items" do + expect { + expect(range).to include(3, 4, 5, 6, 7, 8) + }.to fail_matching("expected #{range} to include 4, 5, 6, 7, and 8") + end + end + + context "with elements that do not support iteration" do + context "with floats" do + let(:range) { 1.0..3.0 } + + it "passes if target includes all items" do + expect(range).to include(1.0, 2.0) + end + + it "fails if target does not include one of the items" do + expect { + expect(range).to include(2.0, 3.0, 4.0) + }.to fail_matching("expected #{range} to include 4.0") + end + + it "fails if target does not include two of the items" do + expect { + expect(range).to include(3.0, 4.0, 5.0) + }.to fail_matching("expected #{range} to include 4.0 and 5.0") + end + + it "fails if target does not include many of the items" do + expect { + expect(range).to include(3.0, 4.0, 5.0, 6.0, 7.0, 8.0) + }.to fail_matching("expected #{range} to include 4.0, 5.0, 6.0, 7.0, and 8.0") + end + end + + context "with times" do + include_context "time range" + + it "passes if target includes all items" do + expect(range).to include(start, start+1) + end + + it "fails if target does not include one of the items" do + expect { + expect(range).to include(start-1, start, start+1) + }.to fail_matching("expected #{range.inspect} to include #{fmt.format(start-1)}") + end + + it "fails if target does not include two of the items" do + expect { + expect(range).to include(start-2, start-1, start) + }.to fail_matching("expected #{range.inspect} to include #{fmt.format(start-2)} and #{fmt.format(start-1)}") + end + + it "fails if target does not include many of the items" do + missing = Array.new(5) { |i| start-i-1} + missing_str = missing[0...-1].map { |time| fmt.format(time) }.join(', ') + ", and #{fmt.format(start-5)}" + + expect { + expect(range).to include(start, *missing) + }.to fail_matching("expected #{range.inspect} to include #{missing_str}") + end + end + end + end + context "for a string target" do it "passes if target includes all items" do expect("a string").to include("str", "a") @@ -457,6 +718,168 @@ class PseudoHash < SimpleDelegator end end + context "for a range target" do + include_context "only runs for modern rubies" + + context "with elements that support iteration" do + let(:range) { 1..3 } + + it "passes if target does not include expected" do + expect(range).not_to include(4) + end + + it "fails if target does include expected" do + expect { + expect(range).not_to include(3) + }.to fail_matching("expected #{range} not to include 3") + end + + context "with exact count" do + it "passes if the block yields the wrong number of times" do + expect(range).not_to include(2).twice + end + + it "fails if the block yields the specified number of times" do + expect { + expect(range).not_to include(2).once + }.to fail_with("expected #{range} not to include 2 once but it is included once") + end + end + + context "with at least count" do + it "fails if the search term is included at least the number of times" do + expect { + expect(range).not_to include(1).at_least(1).times + }.to fail_with("expected #{range} not to include 1 at least once but it is included once") + expect { + expect(range).not_to include(1).at_least(:once) + }.to fail_with("expected #{range} not to include 1 at least once but it is included once") + end + + it "passes if the search term is included too few times" do + expect(range).not_to include(1).at_least(:twice) + end + end + + context "with at most count" do + it "fails if the search term is included at most the number of times" do + expect { + expect(range).not_to include(1).at_most(2).times + }.to fail_with("expected #{range} not to include 1 at most twice but it is included once") + expect { + expect(range).not_to include(1).at_most(:twice) + }.to fail_with("expected #{range} not to include 1 at most twice but it is included once") + end + end + end + + context "with elements that do not support iteration" do + context "with floats" do + let(:range) { 1.0..3.0 } + + it "passes if target does not include expected" do + expect(range).not_to include(4.0) + end + + it "fails if target does include expected" do + expect { + expect(range).not_to include(3.0) + }.to fail_matching("expected #{range} not to include 3.0") + end + + context "with exact count" do + it "passes if the block yields the wrong number of times" do + expect(range).not_to include(2.0).twice + end + + it "fails if the block yields the specified number of times" do + expect { + expect(range).not_to include(2.0).once + }.to fail_with("expected #{range} not to include 2.0 once but it is included once") + end + end + + context "with at least count" do + it "fails if the search term is included at least the number of times" do + expect { + expect(range).not_to include(1.0).at_least(1).times + }.to fail_with("expected #{range} not to include 1.0 at least once but it is included once") + expect { + expect(range).not_to include(1.0).at_least(:once) + }.to fail_with("expected #{range} not to include 1.0 at least once but it is included once") + end + + it "passes if the search term is included too few times" do + expect(range).not_to include(1.0).at_least(:twice) + end + end + + context "with at most count" do + it "fails if the search term is included at most the number of times" do + expect { + expect(range).not_to include(1.0).at_most(2).times + }.to fail_with("expected #{range} not to include 1.0 at most twice but it is included once") + expect { + expect(range).not_to include(1.0).at_most(:twice) + }.to fail_with("expected #{range} not to include 1.0 at most twice but it is included once") + end + end + end + + context "with times" do + include_context "time range" + + it "passes if target does not include expected" do + expect(range).not_to include(start-1) + end + + it "fails if target does include expected" do + expect { + expect(range).not_to include(start) + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)}") + end + + context "with exact count" do + it "passes if the block yields the wrong number of times" do + expect(range).not_to include(start).twice + end + + it "fails if the block yields the specified number of times" do + expect { + expect(range).not_to include(start).once + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)} once but it is included once") + end + end + + context "with at least count" do + it "fails if the search term is included at least the number of times" do + expect { + expect(range).not_to include(start).at_least(1).times + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)} at least once but it is included once") + expect { + expect(range).not_to include(start).at_least(:once) + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)} at least once but it is included once") + end + + it "passes if the search term is included too few times" do + expect(range).not_to include(start).at_least(:twice) + end + end + + context "with at most count" do + it "fails if the search term is included at most the number of times" do + expect { + expect(range).not_to include(start).at_most(2).times + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)} at most twice but it is included once") + expect { + expect(range).not_to include(start).at_most(2).times + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)} at most twice but it is included once") + end + end + end + end + end + context "for a string target" do it "passes if target does not include expected" do expect("abc").not_to include("d") @@ -540,10 +963,111 @@ class PseudoHash < SimpleDelegator }.to fail_matching('expected {:key => "value"} not to include :key') end end - end describe "expect(...).not_to include(with, multiple, args)" do + context "for a range target" do + include_context "only runs for modern rubies" + + context "with elements that support iteration" do + let(:range) { 1..3 } + + it "passes if the target does not include any of the expected" do + expect(range).not_to include(4, 5, 6) + end + + it "fails if the target includes one (but not all) of the expected" do + expect { + expect(range).not_to include(3, 4, 5) + }.to fail_with("expected #{range} not to include 3") + end + + it "fails if the target includes two (but not all) of the expected" do + expect { + expect(range).not_to include(2, 3, 4) + }.to fail_with("expected #{range} not to include 2 and 3") + end + + it "fails if the target includes many (but not all) of the expected" do + expect { + expect(range).not_to include(1, 2, 3, 4) + }.to fail_with("expected #{range} not to include 1, 2, and 3") + end + + it "fails if the target includes all of the expected" do + expect { + expect(range).not_to include(1, 2) + }.to fail_with("expected #{range} not to include 1 and 2") + end + end + + context "with elements that do not support iteration" do + context "with floats" do + let(:range) { 1.0..3.0 } + + it "passes if the target does not include any of the expected" do + expect(range).not_to include(4.0, 5.0, 6.0) + end + + it "fails if the target includes one (but not all) of the expected" do + expect { + expect(range).not_to include(3.0, 4.0, 5.0) + }.to fail_with("expected #{range} not to include 3.0") + end + + it "fails if the target includes two (but not all) of the expected" do + expect { + expect(range).not_to include(2.0, 3.0, 4.0) + }.to fail_with("expected #{range} not to include 2.0 and 3.0") + end + + it "fails if the target includes many (but not all) of the expected" do + expect { + expect(range).not_to include(1.0, 2.0, 3.0, 4.0) + }.to fail_with("expected #{range} not to include 1.0, 2.0, and 3.0") + end + + it "fails if the target includes all of the expected" do + expect { + expect(range).not_to include(1.0, 2.0) + }.to fail_with("expected #{range} not to include 1.0 and 2.0") + end + end + + context "with times" do + include_context "time range" + + it "passes if the target does not include any of the expected" do + expect(range).not_to include(start-3, start-2, start-1) + end + + it "fails if the target includes one (but not all) of the expected" do + expect { + expect(range).not_to include(start-2, start-1, start) + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)}") + end + + it "fails if the target includes two (but not all) of the expected" do + expect { + expect(range).not_to include(start-1, start, start+1) + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)} and #{fmt.format(start+1)}") + end + + it "fails if the target includes many (but not all) of the expected" do + expect { + expect(range).not_to include(start-1, start, start+1, start+2) + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)}, #{fmt.format(start+1)}, and #{fmt.format(start+2)}") + end + + it "fails if the target includes all of the expected" do + expect { + expect(range).not_to include(start, start+1) + }.to fail_matching("expected #{range.inspect} not to include #{fmt.format(start)} and #{fmt.format(start+1)}") + end + end + end + end + context "for a string target" do it "passes if the target does not include any of the expected" do expect("abc").not_to include("d", "e", "f") @@ -872,6 +1396,40 @@ def matches?(_) end end + describe "expect(range).to include(matcher)" do + include_context "only runs for modern rubies" + + it "passes when the matcher matches one of the values" do + expect(10..30).to include( a_value_within(5).of(24) ) + end + + it "fails with a clear message when the matcher matches none of the values" do + expect { + expect(10..30).to include( a_value_within(5).of(4) ) + }.to fail_matching("expected 10..30 to include (a value within 5 of 4)") + end + + it "works with comparison matchers" do + expect { + expect((100..200)).to include(a_value < 90) + }.to fail_matching("expected 100..200 to include (a value < 90)") + + expect(100..200).to include(a_value > 150) + end + end + + describe "expect(range).to include(multiple, matcher, arguments)" do + it "passes if target includes items satisfying all matchers" do + expect("aa".."az").to include(a_string_containing("ab"), a_string_containing('ac')) + end + + it "fails if target does not include an item satisfying any one of the items" do + expect { + expect("aa".."az").to include(a_string_containing("ab"), a_string_containing("abc")) + }.to fail_matching("expected #{("aa".."az").inspect} to include (a string containing 'abc')") + end + end + describe "expect(hash).to include(key => matcher)" do it "passes when the matcher matches" do expect(:a => 12).to include(:a => a_value_within(3).of(10))