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 36 forks source link

#include(object) #43

Open kaievns opened 8 years ago

kaievns commented 8 years ago

Hey @moll, I've got a feature request, if you'd be so kind

Wouldn't it be brilliant if the #include method would match objects agains subset objects? Like for example this:

it('throws 404 error', function* () {
  const response = yield request('/non-existing');
  response.must.include({status: 404, body: 'Not found'});
});

Matching the nested subsets might be a bit tricky, but worth the effort i think.

Thanks!

moll commented 8 years ago

Hey! Thanks for taking the time to think along! Definitely, something akin to that would be useful.

Going further, I think there are a few related possible matchers:

  1. Matching multiple properties together (what you proposed). What now is:

    res.statusCode.must.equal(400)
    res.body.must.equal("Not Found")

    ..could be combined. There's Must.prototype.property for checking a single property. What about using the plural of that for checking more than one strictly (===)?

    res.must.have.properties({statusCode: 404, body: 'Not found'});
  2. A version of the above for checking as Must.prototype.eql does:

    ({name: "Amy", parent: {name: "John"}}).must.have.$some_matcher({parent: {name: "John"}})
  3. A version of include that searches for values in arrays and objects by comparing them as `Must.prototype.eql does:

    [{name: "John"}, {name: "Mike"}].must.$some_matcher({name: "John"})
  4. A version of include that searches for value in arrays and objects by a subset of their values, like point 1 above.

And a few permutations of the above with shallow and deep options. Adding them all would probably get too complicated for anyone to remember. Just listing them out for completeness.

Coming back to your idea, would starting with properties for shallow strict checking fulfill what you had in mind? And possibly one (still shallow) version of it for matching with eql-style for plain and value objects?

kaievns commented 8 years ago

hey, thanks for a quick response!

so, the option number 1. is how i do it at the moment, but it is a bit of a pain, plus it is not going to work with promises (.must.resolve.to.have.property('status', 404); ....?)

but, I like the idea of #properties, that would do the trick i think.

although, #includes is the first thing i'd go for tbh. it works with strings and arrays, so i'd expect it to work with objects as well. but that's just me. as i said, #properties will be fine if you feel strongly about it

moll commented 8 years ago

Morning!

Actually I think you could use the former with promises.

// When using Mocha to return a promise:
return Promise.all([
  obj.must.then.have.property("foo", 1),
  obj.must.then.have.property("bar", 2),
])

// When using Co to drive the generators:
yield [
  obj.must.then.have.property("foo", 1),
  obj.must.then.have.property("bar", 2),
]

Using a single combined matcher will be shorter though. :)

The existing include matcher works with (plain or not) objects, but it just checks if they have a any key with the given value. I try to avoid overloading based on argument types in Must.js to reduce the chance of JavaScript's peculiarities causing bugs. If there were an overload, someone will end up calling obj.must.include(new Date(2000, 5)) and with Dates being empty objects, cause a false positive. That's why I'd add a new matcher for [shallowly] asserting on multiple properties at once.

kaievns commented 8 years ago

makes sense. and i definitely appreciate the thoughtfulness you bring into designing the API!

as for the promises I guess i meant a case that look somewhat like this one:

it('returns the good data back', () => {
  return request('/some/url').must.resolve.to.include({
    status: 200,
    body:  {some: 'data'}
  });
});

I'm not sure if there is a way i can have two assertions in a situation like this one. I normally just use generators or async/await and match both properties separately. But, unfortunately, not everyone is keen on using generators for an async control flow; sometimes you have to fall back to promises. And then I'd have to have an extra step, like .then(res => ({status: res.status, body: res.body}) and match it cleanly. Which, in my view obscures the intent a bit. .resolve.to.include({this and that}) would communicate the requirements more clearly I think

moll commented 8 years ago

Hey again!

I added Must.prototype.properties and Must.prototype.ownProperties (https://github.com/moll/js-must/blob/master/CHANGELOG.md). While they don't yet cover the nested object assertion example you have above ({body: {some: data}}), they should be a start. :)

Would you mind giving it a quick sanity check before I cut a stable release? You can install from here with npm install moll/js-must. Thanks!

kaievns commented 8 years ago

it looks pretty good to me, but i think matching nested params needs to be there. otherwise it will just explode unexpectedly on users

moll commented 8 years ago

Absolutely, a "nesting" matcher will be useful, too. While I don't find the short-is-more-lax pattern (inspired by == vs ===) particularly intuitive, I'm thinking of naming the eql-based matcher props to be consistent for now. Until a better naming scheme comes along at least. What do you think?

kaievns commented 8 years ago

come to think about it, this is a really good idea. then it makes perfect sense