-
-
Notifications
You must be signed in to change notification settings - Fork 397
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
Prevent infinite ranges blowing up matcher descriptions #755
Conversation
rescue IOError # STDOUT is enumerable but `map` raises an error | ||
# STDOUT is enumerable but `map` raises an error | ||
# Range is enumerable except when infinite | ||
rescue IOError, TypeError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of rescueing TypeError
, should we consider changing the definition of enumerable?
so that ranges aren't enumerated? After all, even for an enumerable range (e.g. all ints: 1..10
), I don't think we want it enumerated in the description. We'd rather just have it inspected and displayed as 1..10
, right?
If we go that route, we should perhaps rename enumerable?
to should_enumerate?
or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
Updated |
}.to fail_with(infinite_range.inspect) | ||
end | ||
|
||
it "doesnt enumerate normal ranges" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/doesnt/doesn't/
LGTM. Do you plan to backport this to 3.2? |
Travis doesnt like whatever I did, reopening PR |
Prevent infinite ranges blowing up matcher descriptions, like IO it seems they're not always enumerable. Fixes #754.