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

Make `.throw()` check error instance #18

Closed mstade closed 9 years ago

mstade commented 9 years ago

This adds support for the following assertion, which didn't work before.

var expect = require('must')
   , err   = new Error('Sad face! :o(')

expect(function() { throw instance }).to.throw(err)

Love your coding style btw!

moll commented 9 years ago

Hey! Thanks for the help and sorry for a late reply.

So, on the surface a great addition. I'm just a little worried whether this will make Must.prototype.throw complicated. Currently it's a matcher-style function, with every input doing something special instead of its immediate value. Checking instances of Errors directly would diverge to a special case.

I myself almost never use must.throw and instead do a bit of boilerplate:

var ex
try { someFn() } catch (ex) { err = ex }
ex.must.be.an.instanceof(HttpError)
ex.code.must.equal(404)
ex.message.must.include("Not Found")

That to me is more expressive and generic, especially if you want to test other properties besides message. In that vein, I've been instead thinking of perhaps a helper to simplify that catch block. Perhaps something like:

var err = Must.catch(someFn)
ex.must.be.an.instanceof(HttpError)
ex.code.must.equal(404)

That Must.catch could also be named Must.throw or Must.thrown etc. Your use case would then be something like Must.catch(someFn).must.equal(yourErr).

What do you think?

mstade commented 9 years ago

Oh I like that, good idea! It'd also enable this nifty pattern (which Crockford-ites will hate):

with (Must.catch(someFn)) {
  expect(message).to.equal('oh noes')
  expect(code).to.equal(404)
}

Now, I guess Must and expect are in fact equivalent here, so Must.catch could be expect.error or expect.thrown or something. But that's all bike shedding anyway. It's a good idea for sure. Unfortunately, I don't have time to implement it right now; if I can find a spare moment I'll see what I can do.

moll commented 9 years ago

Thanks for the quick response.

Don't worry about it. I can do that once I get in the flow. ;-) I've been meaning to refactor those tests for a while now. They're a little too verbose. Not to mention other issues here need some additions.

moll commented 9 years ago

I added #19 to continue the expect.error idea. Should we close this for now and later return to it when applicable?

mstade commented 9 years ago

Sure thing!