lt, lte, gt & gte no more work with strings or mongodb ObjectIds

[jlchereau]

In previous versions of chaiJS expect(x).to.be.lt(y) used to work with x and y being strings or mongodb ids.

Since recently, this comparison raises AssertionError: expected 'x' to be a number or a date .

This is a serious limitation, especially when checking the sort order of data arrays returned by RESTful APIs.

[vieiralucas]

Hello @jlchereau, thanks for your issue.

This was a breaking change from chai v3.5 to v4.0. Those assertions stopped working on strings.
You can check release notes for v4.0 here

The assertions: within, above, least, below, most, increase, decrease will throw an error if the assertion's target or arguments are not numbers. (Related Issues: #691, PRs: #692, #796)

The reasons are discussed at #691

About it not working for Dates anymore: It was unintentional and it is fixed for chai v4.1
This code works for me on chai v4.1

const { expect } = require('chai')

expect(new Date('2017-07-27T10:44:49.599Z'))
  .to.be.lt(new Date('2017-07-27T10:44:55.830Z'))

I'll close this issue for now.

Please feel free to keep the discussion here.

Thanks again 😸

[jlchereau]

Thx @vieiralucas.

The reason explained at #691 makes perfect sense but chaiJS now lacks the corresponding assertions for strings (mongodb ids are converted to strings). Would it make sense to consider slt, slte, sgt and sgte for strings athough I appreciate we can still do expect(str1 < str2).to.be.true?

[meeber]

I'm definitely in favor of allowing strings as long as the target and given value are both strings. The mongo objectids part is a bit tougher. I assume they're objects that get coerced to strings during comparison via .valueOf?

[vieiralucas]

I'm definitely in favor of allowing strings as long as the target and given
value are both strings.

Looks good to me, I think that if the other members agree on that too we can reopen this and mark as a pull-request-wanted?

@chaijs/chai

[keithamus]

I'm definitely in favor of allowing strings as long as the target and given
value are both strings.

I don't think I am, lt, gt and co all use >/< operators which means anything other than numbers (and, okay, dates) are relying on implicit coercion, which is probably undesirable as its a minefield.

I understand that @jlchereau you are using them to compare mongo ObjectId - but these IDs are not sequential (they are a hash over time, machineid, pid, and 3 random bytes).

If you're attempting to assert an ObjectId was created before or after another one, then you would be much better served manually extracting the timestamp using ObjectID.getTimestamp() - this will return you a Date which we can rely on for comparison.

[keithamus]

I'll go a little further here - I think if we could do anything it would be to thoroughly document the decisions behind these kinds of errors, and whenever an error like this occurs - link the user to the relevant docs. Regardless of breaking changes - what we've done with lt/gt is make an educated, reasoned decision about testing (which is a good thing) and subsequently withheld that info from our users.

This should be an opportunity for us to educate our users on the pitfalls of running tests that cause these failures.

[jlchereau]

@keithamus one of the use cases I have is checking the sort order of requests against a RESTful API that implements server sorting and paginating (to sort grid columns). Sorting applies on contact last names, city names, country names, ... and needs to be tested. MongoDB ids is just the default sort order in MongoDB.

[keithamus]

@jlchereau I don't know the specifics of your tests but if you're looking at testing particular sorts, then you should make requests with a fixed data set, and assert using .deep.equal on the dataset - this will ensure order is retained.

[meeber]

In general I'm opposed to type coercion in an assertion library. However, in this particular case I think the normal risks associated with type coercion are mitigated.

The main mitigation is that both the target and the given value must both be strings; the same coercion happens to both values using the same string-to-number algorithm. This eliminates the element of surprise that is usually associated with problems that arise from type coercion. To me, the fact that strings are being converted to unicode code points in the background isn't significantly different than dates being converted to milliseconds since the Unix epoch. I'm having trouble thinking of what exactly we're protecting the user against by disallowing string-to-string comparisons.

I am, however, opposed to object-to-object comparisons; the coercion algorithm in that case isn't consistent, and a lot of surprising things could happen.