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

include breaks with a time/float ranges #1191

Open
pirj opened this issue Jun 11, 2020 · 5 comments
Open

include breaks with a time/float ranges #1191

pirj opened this issue Jun 11, 2020 · 5 comments
Labels

Comments

@pirj
Copy link
Member

pirj commented Jun 11, 2020

Subject of the issue

include breaks with a time range

Originally reported by @schwern in rubocop/rubocop-rspec#926

Your environment

  • Ruby version: 2.7.1
  • rspec-expectations version: master (eb1787f)

Steps to reproduce

RSpec.describe 'include' do
  it do # works
    range = (3..5)
    expect(range).not_to include(6)
  end

  range = (Time.now-20...Time.now-10)
  now = Time.now

  it do # works
    expect(range.include?(now)).not_to be true
  end

  it do # breaks
    expect(range).not_to include(now)
  end
end

Expected behavior

All examples pass

Actual behavior

The example with a time range and include matcher fails with:

  1) include is expected not to include 2020-06-11 10:47:01.120378000 +0300
     Failure/Error: expect(range).not_to include(now)

     TypeError:
       can't iterate from Time
     # ./lib/rspec/matchers/built_in/include.rb:134:in `each'
     # ./lib/rspec/matchers/built_in/include.rb:134:in `any?'
     # ./lib/rspec/matchers/built_in/include.rb:134:in `actual_collection_includes?'
@pirj pirj added the bug label Jun 11, 2020
@schwern
Copy link

schwern commented Jun 14, 2020

It breaks on Float as well.

require 'rspec'

describe 'include' do
  it 'is not equivalent' do
    obj = 2.5
    range = (obj...obj)
    expect(range.include?(obj)).to be_falsey  # pass
    expect(range).not_to include(obj)  # TypeError: can't iterate from Float
  end
end

@pirj pirj changed the title include breaks with a time range include breaks with a time/float ranges Jun 16, 2020
bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 4, 2021
This is issue rspec#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)

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
with Ranges previously)
bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 4, 2021
This is issue rspec#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)

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
with Ranges previously)
bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 4, 2021
This is issue rspec#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)

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
with Ranges previously)
bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 4, 2021
This is issue rspec#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)

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
with Ranges previously)
bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 4, 2021
This is issue rspec#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)

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
with Ranges previously)
bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 4, 2021
This is issue rspec#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.
bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 4, 2021
This is issue rspec#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.
@bclayman-sq
Copy link
Contributor

Hi @pirj, @schwern, @JonRowe! I just put out a PR that attempts to address this issue. Would one of you mind giving it a review when you get a chance?

@pirj
Copy link
Member Author

pirj commented Oct 4, 2021

@bclayman-sq Certainly. We're always watching. 👀

bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 4, 2021
This is issue rspec#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.
bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 5, 2021
This is issue rspec#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.
@bclayman-sq
Copy link
Contributor

Hi @pirj, thanks again for kicking off those builds for me!

I've fixed up a couple issues causing a failing build and think this one should pass. In particular, I've now targeted ruby versions >= 2.1.9 for this improvement. Assuming CI passes, I'll update the docs to indicate these improvements are available for >= 2.1.9. How does that sound to you?

bclayman-sq added a commit to bclayman-sq/rspec-expectations that referenced this issue Oct 5, 2021
This is issue rspec#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.
@schwern

This comment was marked as off-topic.

pirj pushed a commit to bclayman-sq/rspec-expectations that referenced this issue Dec 28, 2023
This is issue rspec#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants