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

toBeInRange bug in implementation #670

Open
lcialon opened this issue Nov 7, 2023 · 4 comments
Open

toBeInRange bug in implementation #670

lcialon opened this issue Nov 7, 2023 · 4 comments

Comments

@lcialon
Copy link

lcialon commented Nov 7, 2023

Bug in the implementation of toBeInRange. @mayankshukla94
expect([1,2,3]).toBeInRange(1,3) // This should return pass according to documentation
From doc:
Use .toBeInRange when checking if an array has elements in range min (inclusive) and max (inclusive).

But in the code max value is exclusive.

@SimenB
Copy link
Member

SimenB commented Nov 7, 2023

Not sure if we should update docs to match the implementation or vice versa. I guess I can see the argument for either, but inclusive on both ends makes the most sense to me (I think 😅).

@keeganwitt @mattphillips thoughts?

@mayankshukla94
Copy link
Contributor

@lcialon @SimenB Sure, I'll raise a new PR and make both inclusive.

@keeganwitt
Copy link
Collaborator

Not sure if we should update docs to match the implementation or vice versa. I guess I can see the argument for either, but inclusive on both ends makes the most sense to me (I think 😅).

@keeganwitt @mattphillips thoughts?

Sorry I missed this question. I was trying to think of parallels within JavaScript itself. I'm actually used to the beginning being inclusive, but the end being exclusive. For example, these functions behave that way

As such, I'm actually leaning towards this being a documentation bug.

@keeganwitt
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants