Fix Include Matcher For Ranges

[bclayman-sq]

This PR addresses issue #1191 for ruby >= 2.1.9.

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) Range#succ is 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.

[pirj]

Does it make sense to rebase this on 4-0-dev and get rid of Ruby < 2.3 checks? @JonRowe do we have a timeline for 4.0 release?

[bclayman-sq]

Does it make sense to rebase this on 4-0-dev and get rid of Ruby < 2.3 checks? @JonRowe do we have a timeline for 4.0 release?

👋 Hi @JonRowe, I wanted to follow up on this and to get your review!

[bclayman-sq]

@pirj Thanks for your help on that other PR!

I'm not sure what a normal review timeline is for RSpec; is there something you could do to help get this one approved + merged?

I'd love to get this merged in before the 4.0 release if possible. Thanks again for your help!

[pirj]

It may take a while. We're making our best effort, but often times life and daily job intervenes, and it may take longer. Processing has improved this year, though:

[bclayman-sq]

It may take a while. We're making our best effort, but often times life and daily job intervenes, and it may take longer. Processing has improved this year, though:

Ah that's really helpful; I mostly wanted to be sure it didn't get lost in the shuffle but sounds like these things, understandably, take time. Thanks for the info!

[bclayman-sq]

@JonRowe @pirj 👋 Anything I can do to get this PR merged?

[bclayman-sq]

I'm happy to merge this once we've addressed what happens on older Rubies, I don't mind it "not working", I'd just like the specs to show what happens on older Rubies, even if thats just this (or equivalent):

if RUBY_VERSION >= "2.1.9"
  example.run 
else
  expect { example.run }.to raise_error(WhateverErrorIsRaised)
end

Hi @JonRowe,

I'd like to suggest going with the approach that only runs these new tests for ruby >= 2.1.9. This covers all ruby versions in the last 9 years and fixes the bug. How would you feel about that?

Here's a bit of technical discussion about why I don't think the proposed approach quite works:

I don't think there's a straightforward way to expect examples to raise a TypeError and only a TypeError. This code:

if RUBY_VERSION >= "2.1.9"
  example.run 
else
  expect { example.run }.to raise_error(WhateverErrorIsRaised)
end

actually causes 2 failures for specs run on pre-2.1.9 ruby:

Got 1 failure and 1 other error:

20.1) Failure/Error: expect(range).not_to include(2.0).twice

TypeError:
  can't iterate from Float
# ./spec/rspec/matchers/built_in/include_spec.rb:797:in `block (7 levels) in <top (required)>'
# ./spec/rspec/matchers/built_in/include_spec.rb:127:in `block (4 levels) in <top (required)>'
# ./spec/rspec/matchers/built_in/include_spec.rb:127:in `block (3 levels) in <top (required)>'

20.2) Failure/Error: expect { example.run }.to raise_error(TypeError)
  expected TypeError but nothing was raised
# ./spec/rspec/matchers/built_in/include_spec.rb:127:in `block (3 levels) in <top (required)>'

This is because the original expectation is evaluated first and isn't met (it raises a TypeError). Then the second expectation also isn't met because it expects a TypeError to be raised and this has already happened.

Do you know a way around this? If not, I'd love to merge as is since it still represents a solid improvement. Let me know what you think!

[JonRowe]

This has been migrated to the monorepo, given that the blockers were largely legacy rubys that RSpec 4 won't support I hope to merge this in relatively quickly, sorry it got forgotten about