search

tj / should.js

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

Let instanceof() accept wrapped primitives #157

Closed stimberm closed 10 years ago

stimberm commented 10 years ago

I want to fix #156 by having instanceof() be optimistic in the case of primitive values, in order to compensate for should() invoking valueOf()

Note that type() is currently also optimistic:

var five = Object(5);
five.should.have.type('number'); // works
(typeof five).should.equal('number');  // throws
btd commented 10 years ago

LGTM, but pls add in test Boolean and String for consistency. Also I am not sure what you mean about .type.

alsotang commented 10 years ago

This pull request should be reject.

Since in should.js L23 - L25:

var should = function(obj) {
  return new Assertion(util.isWrapperType(obj) ? obj.valueOf(): obj);
};

Should.js extract the wrappertype from obj specially, but this PR wrap the value again.

I think it would make should.js unconsistency.

And more, if want optimistic assertion, use .Number.

btd commented 10 years ago

It does not wrap primitive back. Object function wrap primive value to according wrapper, but do not touch reference types. instanceof is useless with primitives in any case.