New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should.js for the browser #26

Closed
NV opened this Issue Nov 23, 2011 · 32 comments

Comments

Projects
None yet
9 participants
@NV

NV commented Nov 23, 2011

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

This comment has been minimized.

Show comment
Hide comment
@tj

tj Nov 23, 2011

Owner

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

Owner

tj commented Nov 23, 2011

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

This comment has been minimized.

Show comment
Hide comment
@weepy

weepy Nov 24, 2011

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

weepy commented Nov 24, 2011

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

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Nov 24, 2011

Owner

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

Owner

tj commented Nov 24, 2011

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

@paulmillr

This comment has been minimized.

Show comment
Hide comment
@paulmillr

paulmillr Nov 27, 2011

Safari 5.1 -- everything works OK for me.

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

👍 to this issue.

paulmillr commented Nov 27, 2011

Safari 5.1 -- everything works OK for me.

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

👍 to this issue.

@jgonera

This comment has been minimized.

Show comment
Hide comment
@jgonera

jgonera Nov 30, 2011

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().

jgonera commented Nov 30, 2011

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

This comment has been minimized.

Show comment
Hide comment
@tj

tj Nov 30, 2011

Owner

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

Owner

tj commented Nov 30, 2011

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

This comment has been minimized.

Show comment
Hide comment
@jgonera

jgonera Nov 30, 2011

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 Nov 30, 2011

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

This comment has been minimized.

Show comment
Hide comment
@jgonera

jgonera Nov 30, 2011

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.

jgonera commented Nov 30, 2011

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

This comment has been minimized.

Show comment
Hide comment
@weepy

weepy Nov 30, 2011

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:
#26 (comment)

weepy commented Nov 30, 2011

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:
#26 (comment)

@jgonera

This comment has been minimized.

Show comment
Hide comment
@jgonera

jgonera Nov 30, 2011

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?

jgonera commented Nov 30, 2011

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

This comment has been minimized.

Show comment
Hide comment
@tj

tj Nov 30, 2011

Owner

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

Owner

tj commented Nov 30, 2011

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

This comment has been minimized.

Show comment
Hide comment
@geddski

geddski Dec 2, 2011

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

geddski commented Dec 2, 2011

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

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Dec 2, 2011

Owner

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

Owner

tj commented Dec 2, 2011

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

@jgonera

This comment has been minimized.

Show comment
Hide comment
@jgonera

jgonera Dec 2, 2011

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)?

jgonera commented Dec 2, 2011

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

This comment has been minimized.

Show comment
Hide comment
@tj

tj Dec 2, 2011

Owner

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

Owner

tj commented Dec 2, 2011

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

This comment has been minimized.

Show comment
Hide comment
@jgonera

jgonera Dec 2, 2011

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.

jgonera commented Dec 2, 2011

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

This comment has been minimized.

Show comment
Hide comment
@geddski

geddski Dec 2, 2011

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.

geddski commented Dec 2, 2011

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

This comment has been minimized.

Show comment
Hide comment
@tj

tj Dec 2, 2011

Owner

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

Owner

tj commented Dec 2, 2011

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

This comment has been minimized.

Show comment
Hide comment
@logicalparadox

logicalparadox Dec 15, 2011

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.

logicalparadox commented Dec 15, 2011

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

This comment has been minimized.

Show comment
Hide comment
@rauchg

rauchg commented Dec 15, 2011

FWIW, here's my implementation

https://gist.github.com/ad4ced5c2ecd5fb50eb2

@rauchg

This comment has been minimized.

Show comment
Hide comment
@rauchg

rauchg Dec 15, 2011

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

rauchg commented Dec 15, 2011

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

This comment has been minimized.

Show comment
Hide comment
@weepy

weepy Dec 15, 2011

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:
#26 (comment)

weepy commented Dec 15, 2011

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:
#26 (comment)

@c4milo

This comment has been minimized.

Show comment
Hide comment
@c4milo

c4milo Dec 16, 2011

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

c4milo commented Dec 16, 2011

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

@geddski

This comment has been minimized.

Show comment
Hide comment
@geddski

geddski Dec 17, 2011

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.

geddski commented Dec 17, 2011

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

This comment has been minimized.

Show comment
Hide comment
@c4milo

c4milo Dec 17, 2011

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?

c4milo commented Dec 17, 2011

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

This comment has been minimized.

Show comment
Hide comment
@tj

tj Dec 17, 2011

Owner

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

Owner

tj commented Dec 17, 2011

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

@c4milo

This comment has been minimized.

Show comment
Hide comment
@c4milo

c4milo Dec 17, 2011

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

c4milo commented Dec 17, 2011

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

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Dec 17, 2011

Owner

hmm could be chai

Owner

tj commented Dec 17, 2011

hmm could be chai

@logicalparadox

This comment has been minimized.

Show comment
Hide comment
@logicalparadox

logicalparadox Dec 17, 2011

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

logicalparadox commented Dec 17, 2011

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

@c4milo

This comment has been minimized.

Show comment
Hide comment
@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Jan 6, 2012

Owner

closing this since we have chai

Owner

tj commented Jan 6, 2012

closing this since we have chai

@tj tj closed this Jan 6, 2012

@c4milo

This comment has been minimized.

Show comment
Hide comment
@c4milo

c4milo commented Jan 6, 2012

le chai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment