tj / should.js

BDD style assertions for node.js -- test framework agnostic
MIT License
2.75k stars 195 forks source link

way to specify an additional message to a should? #1

Closed dominictarr closed 13 years ago

dominictarr commented 14 years ago

sometimes it is necessary...

tj commented 13 years ago

you can still use should like assert, so perhaps:

should.equal('foo', 'bar', 'failed !!! ');

but I dont have a good solution at the moment. Will look into it

dominictarr commented 13 years ago

what about something like this:

describe(obj,"any object").should.not.have.ownProperty('should')

this could handle null as well,

describe (func(),"return value of func()").should.not.be.null

which would print out a message: reutrn value of func() should not be null

hmm. with this syntax, .should wouldn't need to be a monkeypatch.

tj commented 13 years ago

well I would still want to keep .should it looks a lot better. this would be possible, but verbose as well:

foo.should.equal('bar').with.message('oh noes')

if we defer assertions, otherwise it would have to be something even more awkward:

foo.should.message('oh noes').equal('bar')
dominictarr commented 13 years ago

yes, and of course you might not want a message.

deferring assertions would make a nice syntax, but I can't think of a simple implementation (assert in nextTick, or at next call of .should?)

or maybe a setter:

rule = foo.should.equal('bar').with.message('oh noes')
rule() // or rule.check() 

execution should stop when an assertion should fail. If it kept on executing I would expect hard to diagnose errors... & the whole point of something like this is to make good error messages...

but also, .with.message("whatever") doesn't draw you in to documenting well, but with describe(obj,"a toy example").should.be.a('object') actually prompts you to enter something useful. and you can also format it like this:

var it = 
  describe(obj,"a toy example")
it.should.be.a('object')
it.should.have.property('silly name')

if describe only added a __description_for_should property to the object then we could keep .should working the same, not need describe if we didn't want it.

dominictarr commented 13 years ago

okay, I've hacked up a quick implementation of describe() on my fork. https://github.com/dominictarr/should.js/tree/add_describe

describe(obj,"description") returns an object which wraps obj, and has a should property which creates an assertion around obj, with a description. i've modified Assertion to show add this to the message.

should itself hasn't changed and you can still .should anything which isn't null

aseemk commented 13 years ago

I'm using should.js for the first time -- just started changing our current tests from assert to should -- and I immediately saw this missing too.

I was thinking an easy and clean way to support messages would be simply to have an optional parameter to all shoulds, e.g.:

foo.should.equal(bar, 'when reading from baz');

With the way should.js is written currently, this would only be possible on assertion methods like equal() and not on assertion properties like ok, but I already opened issue #9 to support all assertions as methods.

What do you guys think? This looks a lot cleaner and more like standard assertions -- but with should.js's nice syntax -- than the describe() way above.

I've forked this, ready to try my hand at this, so will keep you guys posted if I get this.

Cheers, Aseem

dominictarr commented 13 years ago

aseemk, suggestion look quite reasonable, although there are some assertions which take optional parameters, for example [].should.have.property('length') checks that there is a length property, but not what it is.

so if you wanted to pass a message, you couldn't pass it in every situation.

what you are suggesting would work for most cases though.

aseemk commented 13 years ago

I don't understand. Why couldn't you still pass a message? [].should.have.property('length', 'someone effed with Array.prototype!');

dominictarr commented 13 years ago

sorry, I left out half of my post. .property takes an optional second argument which checks the value of the property.

for example [].should.have.property('length',0). so, if you passed in a message it would think it was value of the property.

AssertionError expected: length == 'someone effed with Array.prototype!' but got: 0

you would have to say [].should.have.property('length', 0, 'someone effed with Array.prototype!');

aseemk commented 13 years ago

I haven't gotten here yet, but I assume this is just a method overload -- the implementation would just look at the number of arguments present.

EDIT: oops, I get what you're saying now. Good point! I'll think about this.

dominictarr commented 13 years ago

hmm. maybe add another method:

.defined(name,message)

which only checks if there is a property named name, but does not check what the property is.