style-guide
style-guide copied to clipboard
jshttp
stop using should.js
i'm getting pretty annoyed with it and it's pretty unnecessary with node's assert
I have no formal opinion other than that I have never really used should.
i think not using should is fine. we should simply make it a rule on new things, rather than changing all the existing stuff. there are cases where an error will produce a less-desirable error message, but i think it outweighs altering globals within testing
yep new things. my main issue is that they keep removing features so a lot of my stuff is on super old versions.
I would recommend taking a look at chai/should before going back to assert. I find writing tests to be much quicker with assertion libraries that provide a fast way to do type checking, range checking, etc. The error messages are also a lot more useful (time saver). Another plus it that it integrates nicely with sinon which is a valuable tool for any javascript test suite.
If you have an issue with 'should' modifying the prototype then you could always go with chai/expect. I personally prefer should over expect (pure preference). The only property on Object.prototype that gets modified is 'should'. I'd like to avoid messing around with the prototype but pragmatically speaking this doesn't have any real world consequences (unless in IE). Conflicts with properties named "should" shouldn't really happen, the word 'should' isn't very descriptive as a property or function name. If there ever was a property name conflict then I'd say its worth rethinking the name of that property being tested or using the alternative should() wrapper.
@dougwilson I haven't tested it in node 0.6 but it seems to support down to node 0.4
@jonathanong at least its a dev dependency :stuck_out_tongue:
but it seems to support down to node 0.4
yes, i saw that, but many modules these days lie. the module is not actively tested on node 0.6 on travis ci, so it very well does not actually work. assert, on the other hand, does work since it's part of core and exists in 0.6, which is yet another reason i prefer it.
i agree, using it can lead to non-useful assertion messages, but it works fine, is one less thing to keep up-to-date and break on us, and the assert library even follows an actual spec, so what it provides us is actually stable.
@dougwilson good point. I wouldn't use it for any projects that must support node 0.6.
haha, sure. the saddest part is that they use travis ci, but _only_test on 0.10, while our base line of support is 0.8, 0.10, and 0.11, with 0.6 when possible.
@esco I noticed you got over here from express-paginate project. I don't personally know anything about it, and it looks like it sadly doesn't even test on 0.8, anyway. It certainly doesn't seem to live up to expressjs org standards from that point for whatever reason. Anyway, this issue is of course mainly of things like the jshttp org, though it can influence decisions in other orgs by the same people :)
@dougwilson express-paginate is a fairly new project that I discovered at a meetup. They just added travis today, not sure why 0.8 support was dropped. It looks like chai dropped 0.6 and 0.8 support explicitly. With that said, it looks like it wouldn't be a good fit as long as 0.8 is in your base line or if your future support goals don't align with the chaijs team.
Ah, good to know they dropped node.js 0.8 and such explicitly. Now you know why I said the engine in the package.json usually lies, because people just don't seem to care about keeping it up-to-date :) 0.8 is 100% required to support, at least until 0.12 comes out. 0.10.31 itself still has memory leaks that are sitting as issues in the node repo just ignored, but 0.8 does not have the issues (they were all regressions between 0.8 and 0.10). In my organization we really cannot even use 0.10 because of some of these memory leaks.
@dougwilson ha ha yeah they should update that. Looks like assert is your best option in terms of support.
If we're talking about tests... Is there any good way to define each assert as a separate test without it ('blahblah', function() { assert() }) boilerplate?
@rlidwka if the tests are basically the same, just do them all in one it?