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

Range assertions #119

Open
igorwojda opened this issue Nov 8, 2018 · 4 comments
Open

Range assertions #119

igorwojda opened this issue Nov 8, 2018 · 4 comments

Comments

@igorwojda
Copy link

igorwojda commented Nov 8, 2018

I was thinking about adding range specific assertions.

First of all current shouldContain / shouldNotContain may not be efficient as we are using generic inferable implementation withforEach under the hood instead of range specificrange.first and range.lastproperties (r1.first >= r2.first && r1.last <= r2.last)

Second, below generic iterable extensions seems to be a bit counter-intuitive because we are not treating ranges as mathematical sets https://en.wikipedia.org/wiki/Set_(mathematics) :

val r1 = 1..9
val r2 = 4..5

//should pass because r2 range contains r1 (currently this fails)
r1 shouldContain r2 
//java.lang.AssertionError: Iterable doesn't contain "4..5"
//   Expected:   the Iterable to contain "4..5"
//   Actual:     1, 2, 3, 4, 5, 6, 7, 8, 9

//--------
// should fail because r2 does not contain r1 (now it fails but kind of from wrong reason - looks 
//for r1 object, not for it's values - mathematical set)
r2 shouldContain r1
//java.lang.AssertionError: Iterable doesn't contain "1..9"
//  Expected:   the Iterable to contain "1..9"
//  Actual:     4, 5

Maybe we should add more specific implementation for shouldContain/shouldNotContain to Range related types (ClosedRange children) 🤔 or optionally add range specifics extension shouldBeInRange / shouldNotBeInRange

val r1 = 1..9
val r2 = 4..5

r1 shouldBeInRange r2 //fail
r2 shouldBeInRange r1 //pass
@MarkusAmshove
Copy link
Owner

Thanks for raising this, that would be a nice addition!

A specific overload of either function should do it, but I'm not sure which function we should use.

shouldContain would be intuitive because it was the first thing you tried. However I think we wouldn't be able to use shouldContain for e.g. a list if ranges anymore and therefore could break existing code.

shouldBeInRange might not be the first thing I'd think about when thinking about Sets, but might show a clearer intention

I would go with the 2nd option, but if you want to implement it I'm fine with either :)
In the case of the first option we should make sure not to break existing code

javatarz added a commit to javatarz/Kluent that referenced this issue Oct 5, 2019
Also add JVM support for backtick version
@javatarz
Copy link
Contributor

javatarz commented Oct 13, 2019

I've added shouldBeInRange for ranges. The original issue said we want shouldNotContain. Would a matching shouldNotBeInRange/shouldBeOutOfRange make more sense?

Please let me know which one you'd prefer @MarkusAmshove and @igorwojda. I'll update my PR accordingly :)

@MarkusAmshove
Copy link
Owner

I'd vote for shouldNotBeInRange to stay consistent with other assertions :-)

@igorwojda
Copy link
Author

igorwojda commented Oct 14, 2019

Same with me

Kshitij09 added a commit to Kshitij09/Kluent that referenced this issue Oct 20, 2019
Signed-off-by: Kshitij09 <kshitijpatil98@gmail.com>
javatarz added a commit to javatarz/Kluent that referenced this issue Jan 15, 2020
MarkusAmshove added a commit that referenced this issue Jan 18, 2020
Add shouldBeInRange for ClosedRanges #119
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants