search

busterjs / buster

Abandoned - A powerful suite of automated test tools for JavaScript.
http://docs.busterjs.org
Other
448 stars 37 forks source link

Replace tmpvar/jsdom with cheeriojs/cheerio? #456

Closed mroderick closed 6 years ago

mroderick commented 9 years ago

Recently, contextify has started failing to build on node@0.6 on both Linux and OSX.

The only dependency on contextify that I can find Buster modules, is in tmpvar/jsdom, which only supports iojs since v4.0, so we won't be able to upgrade ourselves out of this problem or any future problems with jsdom. Even if/when node and iojs merges, it's clear that jsdom project is not interested in maintaining compatibility with legacy node versions (which is a valid choice).

With that in mind, I think it is too much effort to get contextify building on node@0.6, only to be stuck with a jsdom dependency that we can no longer upgrade. I've looked around, and while there are forks of jsdom for node, none seem to be that well maintained.

Also: jsdom doesn't seem to work well in node@0.12, as you can see in my PR to run tests in more node versions

Would you consider merging a pull request that replaces the use of tmpvar/jsdom with cheeriojs/cheerio?

Also, why are there two html reporters? (I'd like to avoid duplication of effort, if I can)

dwittner commented 8 years ago

@mroderick, if cheeriojs/cheerio fits all our needs, there is nothing against it. Does cheeriojs/cheerio needs node-gyp for installation? If not, this would be an advantage too.

There are two html reporters, because html2 is a new, stylish version created by @cjohansen. I am not sure, but i guess the html reporter will be thrown away or replaced by html2 reporter some day.

dominykas commented 8 years ago

cheerio won't help in buster-test - the way jsdom is used, is to construct a full browser-like environment, and then regular DOM functions are being used to build the reporter output. cheerio only provides a jquery-like API to deal with HTML. It might just be enough to build the output, but it would also mean that a compatible module would be needed on the browser side and I'd really hate to add jquery itself as a dependency.

Investigating alternatives...

dominykas commented 8 years ago

Some alternatives I'm thinking about:

mroderick commented 8 years ago

I think we should just get rid of it. That will benefit most users

dominykas commented 8 years ago

Wait, actually, jsdom@3.x series seem to work in all nodes, so that could be a stopgap solution, although I'm in favour of just removing it to speed up install - not sure it really brings value being there.

dominykas commented 8 years ago

A little bit late to the party, contextify actually released a 0.1.15 some 10 days ago - which luckily allowed me to upgrade jsdom to v3 in buster-test, which I will release as soon as all Travis builds pass (something odd with npm install is breaking them occasionally...) - so that solves the immediate problem. As for the future - we'll see :)

Upd 1: released buster-test@0.7.14 which should now work Upd 2: have a working travis build for both - jsdom 3 (compat) and 7 (latest): https://travis-ci.org/busterjs/buster-test/builds/92694238 Upd 3: this is actually not a bad way of doing things: https://github.com/busterjs/buster-test/pull/32/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R73 - if someone is on pre-LTS and they are using html reporter on node side, then they can install jsdom@3 locally in their project and npm will be content with it, i.e. jsdom@7 will not be installed and will not be used