moll / js-must

An assertion library for JavaScript and Node.js with a friendly BDD syntax (awesome.must.be.true()). It ships with many expressive matchers and is test runner and framework agnostic. Follows RFC 2119 with its use of MUST. Good stuff and well tested.
Other
336 stars 35 forks source link

Add possibility to use custom equality comparator in #61

Open bennidi opened 7 years ago

bennidi commented 7 years ago

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})
moll commented 7 years ago

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?

bennidi commented 7 years ago

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.

moll commented 7 years ago

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/)? ^_^

bennidi commented 7 years ago

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 :)

moll commented 7 years ago

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.

bennidi commented 7 years ago

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.

moll commented 7 years ago

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.

bennidi commented 7 years ago

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".

dzek69 commented 7 years ago

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.

dietergeerts commented 6 years ago

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()