tj / should.js

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

Should.js for the browser #26

Closed NV closed 12 years ago

NV commented 12 years ago

I would like to use Mocha BDD and run specs in the browser. Would be nice to have a build of should.js that doesn't rely on Node's stuff.

tj commented 12 years ago

the design of should is pretty specific to modern js implementations that support accessors etc, my co-worker guillermo is almost done a nice option though using expect(foo).to.equal(bar) etc, so it's similar

weepy commented 12 years ago

anyone actually using mocha is going to be using a modern js implementation.

tj commented 12 years ago

not necessarily, even modern safari has issues with keywords as props foo.should.be.true etc will fail

paulmillr commented 12 years ago

Safari 5.1 -- everything works OK for me.

> a = {true: false, if: true, while: true}
Object
> a.while
true

:+1: to this issue.

jgonera commented 12 years ago

Alternatively, you could use Object.defineProperty to define those few keyword properites, e.g.:

Object.defineProperty(Assertion.prototype, 'true', { get: function() {
  this.assert(
      true === this.obj
    , 'expected ' + this.inspect + ' to be true'
    , 'expected ' + this.inspect + ' not to be true');
  return this;
}});

I think there might still be a problem with instanceof() method, but it could be renamed to instanceOf(). I know it's meant to resemble the operator, but I guess it's also not a bad idea to make it consistent with lengthOf().

tj commented 12 years ago

Object.defineProperty isn't available in all browsers, though I'm sure it would be fine for modern browsers if that's all you care about

jgonera commented 12 years ago

For me supporting relatively modern browsers would be enough. However, I've just tested it and even though I can define a property called true with Object.defineProperty in some older version of JavaScriptCore (Safari's engine), it will still throw a SyntaxError when I try to use it in a test (something.should.be.true).

Still, this makes should.js run in older browsers (they will parse the JS without problems). The test itself must use alternative syntax though (something.should.equal(true)). The only problem is that chaining would require some alternative word instead of with, maybe having or that.has.

Actually, I'm working on making should.js work in PhantomJS which uses JavaScriptCore and I encountered another weirdness:

'test'.should.equal('test')

Results in:

AssertionError: expected { '0': 't', '1': 'e', '2': 's', '3': 't' } to equal 'test'

This seems to affect primitive types only. The lame fix I came up with is:

Object.defineProperty(Object.prototype, 'should', {
  set: function() {},
  get: function() {
    // TODO: who knows why this is needed in JavaScriptCore
    // if not applied, "test" becomes { '0': 't', '1': 'e', '2': 's', '3': 't' }
    // and 123 becomes {}
    if (this instanceof String || this instanceof Number || this instanceof Boolean) {
      return new assert.Assertion(this.constructor(this));
    }
    return new assert.Assertion(this);
  },
  configurable: true
});

If you are interested in merging those hacks, I can make a pull request.

jgonera commented 12 years ago

Having some free time I've made those modifications that make should.js work on JavaScriptCore:

I tried to modify as little code as possible. Nothing should be broken, all the tests pass. It should be easy to load it in a browser now using RequireJS.

weepy commented 12 years ago

one advantage of "expect(x).to.eql(4)" over x.should.eql(4) is that it can handle the case where x is null (as should cannot bind null or undefined ^n^ )

On Wed, Nov 30, 2011 at 5:11 PM, Juliusz Gonera < reply@reply.github.com

wrote:

Having some free time I've made those modifications that make should.js work on JavaScriptCore:

I tried to modify as little code as possible. Nothing should be broken, all the tests pass. It should be easy to load it in a browser now using RequireJS.


Reply to this email directly or view it on GitHub: https://github.com/visionmedia/should.js/issues/26#issuecomment-2962695

jgonera commented 12 years ago

True, but you can also check should.exist(x) first. Anyway, I would like to be able to use both. BTW, is the "expect" version/patch of this library available somewhere already?

tj commented 12 years ago

yeah, that's a slight advantage, though it will break on null/undefined anyway, so it's essentially the same just not a nice message :) it's worth it IMO

geddski commented 12 years ago

Shoot. Without should.js working in the browser it's not a very viable alternative to Jasmine (if you're testing client code).

tj commented 12 years ago

should.js isnt a requirement at all, you can use any assertion lib you want

jgonera commented 12 years ago

I guess we're talking about mocha now ;) I think it would be nice if should.js worked in browsers too. I don't know how the browser version of mocha works, but how about making both of them AMD-compatible?

Also, would you mind if I made a pull request of those two commits I mentioned (I mixed up names/urls in the previous message but I fixed it)?

tj commented 12 years ago

meh I'm not a fan of AMD, nor do I want to restrict their use to AMD. It wouldn't be difficult to make should work browsers, but with the current style of the lib it would be restricted to modern browsers

jgonera commented 12 years ago

As I said, I'm in favor of making it work only in modern browsers. As for AMD, many libraries detect it and work both with and without it (e.g. jQuery), also without much additional work.

geddski commented 12 years ago

Modern browsers would be better than no browsers. I really like should and would love to use it in the browser.

Speaking of AMD, its only a couple lines of code to optionally register as AMD like jQuery, backbone, and underscore do. Probably a separate issue though.

tj commented 12 years ago

yeah im sure someone could have a generic AMD wrapper script thingy, as for making should work in the browser, we could add the little script i use in jade and some others, or @guille created https://github.com/learnboost/browserbuild we could run it through that and see how it goes

logicalparadox commented 12 years ago

I went ahead and made an assert implementation that works for both node.js and browser. Its at https://github.com/logicalparadox/chai/. Has a should interface which is completely API compatible, or your can use expect() if you want your tests to run on both sides.

rauchg commented 12 years ago

FWIW, here's my implementation

https://gist.github.com/ad4ced5c2ecd5fb50eb2

rauchg commented 12 years ago

I have docs et all but they're in the hard drive with the backup from my laptop that was in the fire, I'll recover it soon

weepy commented 12 years ago

Check if the value is truthy:  this.obj == true

should this be:  !!this.obj == true

?

On Thu, Dec 15, 2011 at 6:02 PM, Guillermo Rauch reply@reply.github.com wrote:

I have docs et all but they're in the hard drive with the backup from my laptop that was in the fire, I'll recover it soon


Reply to this email directly or view it on GitHub: https://github.com/visionmedia/should.js/issues/26#issuecomment-3166248

c4milo commented 12 years ago

+1 for having should.js work at least in modern browsers.

geddski commented 12 years ago

In the meantime I'm using chai.js and its BDD style is pretty good. It also has the should syntax but not for the browser either.

c4milo commented 12 years ago

I'm using it too but having troubles with the html reporter, it just says "Script error" whenever an assertion throws. I'm not sure if that's the normal behavior though. @visionmedia any idea?

tj commented 12 years ago

c4milo with mocha + chai? not sure what we're talking about

c4milo commented 12 years ago

ops, yes, mocha + chai. Sorry I should just open an issue in chai.

tj commented 12 years ago

hmm could be chai

logicalparadox commented 12 years ago

@c4milo please post up your specs on chai issues and we can see whats going on

c4milo commented 12 years ago

@logicalparadox https://github.com/logicalparadox/chai/issues/4

tj commented 12 years ago

closing this since we have chai

c4milo commented 12 years ago

le chai