js-must icon indicating copy to clipboard operation
js-must copied to clipboard

Add possibility to use custom equality comparator in

Open bennidi opened this issue 9 years ago • 10 comments

I have recently been using the .have.properties() assertion a lot and in all my use cases I needed a value based comparison (deep equals) instead of ===

I would suggest to add an optional parameter to the properties method that will take a custom comparison function. The default value could be an implementation that uses '==='.

This is how it would look in coffeescript (sorry, for being too lazy to translate into ES)


isEqual = (a,b) -> a is b

(props, equals= isEqual) ->
  obj =@actual
  ok = @actual isnt null

  if ok then for key,value of props
      ok = key of obj and equals obj[key], props[key]
      if not ok then break
  @assert(ok, "have properties", {expected: props, diffable: true})

bennidi avatar Mar 08 '17 08:03 bennidi

Hey! Thanks for sharing your thoughts!

I've had that need occasionally myself, too, and have had an idea to add a Must.prototype.props that internally uses the same equality algorithm that Must.prototype.eql does (also covered in https://github.com/moll/js-must/issues/43). Would that cover your case?

moll avatar Mar 08 '17 08:03 moll

I believe that it would cover my case. However, I like the approach of customization more than introduction of different flavours of equal, eql, properties, props etc. Taking inspiration from libraries like lodash or react I think that the pattern of allowing customization of internal parts of an algorithm covers more cases and leaves more control with the user. Providing sensible defaults also allows to maintain backward compatibility.

bennidi avatar Mar 08 '17 10:03 bennidi

That's true, but in this case wouldn't you say making it customizeable would make it more convoluted for the reader? The Must.prototype.properties matcher is, after all, equivalent to the 3 lines below, and that way any custom equivalence requirement could be written explicitly:

obj.a.must.equal(1)
obj.b.must.equal(2)
obj.c.must.equal(3)

While we're talking about equality, which I'd say is so often done so wrong in JavaScript codebases, have you seen my Egal.js (https://github.com/moll/js-egal/)? ^_^

moll avatar Mar 08 '17 13:03 moll

In my scenario I am testing REST APIs, so I am comparing JSON structures. When I POST an entity to an endpoint and it returns me the stored representation of it, then I want the result to have the same structure as the original. That would be very simple to do if properties() used deep value equality.

I don't object introducing .props() method as it solves me usecase. It doesn't satisfy me own idea about API design though :)

bennidi avatar Mar 08 '17 15:03 bennidi

I'll do props for sure, but coming back to testing your responses, have you thought of just doing body.must.eql({a: 1, b: 2})? Where do the other properties come from that you don't want to check?

My controller tests are usually like this (ignore the generator stuff):

var model = yield db.create({accountId: this.account.id, title: "Work"})
var res = yield request("/activities/" + model.id)
res.statusCode.must.equal(200)
res.body.must.eql(serialize(model))

The serialize function I export from the controller and test that separately as a unit test. That way I can without worry use that to check controller responses.

moll avatar Mar 08 '17 15:03 moll

That approach only works if the controller does not add or filter properties. The assumption that the database representation is structurally identical to the REST resource representation does not hold in some (many?) cases.

bennidi avatar Mar 08 '17 15:03 bennidi

Yep, my controller responses differ from the database or model instances too — that's why I mentioned theserialize function. If, however, the controller adds properties above and beyond the functional serialize, I usually do:

res.body.must.eql({__proto__: serialize(model), addedAttribute: 123})

I wouldn't do prototypical extension that way in the browser, but with finite runtime environments (like Node.js on V8), __proto__ is a great "standardized" feature.

moll avatar Mar 08 '17 15:03 moll

Hmmm. I like it. Do you have some gists or other guides where you show a bit more about how you use must.js in production. I bet you have more "tricks".

bennidi avatar Mar 08 '17 16:03 bennidi

If I can add my thoughts - I'm against adding possibility to globally overrite the matcher.

Methods returning different values for same arguments but different environment configuration (that's what overriding matcher would be) is very confusing and misleading. Have you even wrote something in PHP near 5.0 - 5.4 versions? Hell. I know, they had (probably still have) A LOT of things that depended on server configuration that could break the code and you only want to add one, but still - it's smelly for me.

dzek69 avatar Apr 08 '17 20:04 dzek69

Will this be added in the near feature? I am needing this. For the moment, I use https://www.npmjs.com/package/is-subset with to.be.true()

dietergeerts avatar Feb 23 '18 20:02 dietergeerts