cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Simplify browser testing #300

Closed briancavalier closed 10 years ago

briancavalier commented 10 years ago

This switches to plain CommonJS for test files by browserifying them at test time. It removes test-support in favor of a custom test runner based on code in test-support. It also updates to buster 0.7.x and jshint 2.x


(Original message) This is only for discussion right now. The current browser testing setup is pretty complex. It works great, but requires extra machinery (curl, curl config, a run.js that does some curl-privileged stuff), what feels like an overly involved buster config, and extra boilerplate in each test file.

It would be great to simplify. Looking for feedback on this general direction, suggestions, contributions :D, etc.

For this experiment, I converted 2 tests (inspect-test and join-test) to "plain" commonjs--no boilerplate, except one sniff for window.buster vs require('buster'), which we may also be able to eliminate via browserify's -x switch. npm run browser-test builds those into a single file: test/browser/tests.js and then runs buster-static.

This seems to simplify things a lot. There's no need for curl or curl config, or the curl-privileged hoop jump. It also gets rid of the amd path/moduleid boilerplate.

However, this means the current test-support saucelabs setup doesn't work. I didn't dig into making saucelabs work yet, but will soon.

Also, I switched to es5-shim/sham out of laziness. We could probably browserify poly and get that to work.

cc @scothis @unscriptable @jenius @jaredcacurak

briancavalier commented 10 years ago

I converted this branch to use the new approach for all tests, and re-enabled CI for browser tests. It's all working. I'm going to merge this today unless anyone objects, as it has some nice aspects:

  1. It works. Saucelabs testing is completely busted in master/dev atm, and trying to debug it has been a nightmare.
  2. No test boilerplate. Tests can be authored as plain cjs/node modules.
  3. Browser tests run more quickly. Probably several reasons: updated wd selenium driver and sauce-connect (also fixes heartbleed), and browserifying the tests means 1 script tag, instead of many.
  4. No AMD loader. No need for curl config, or reaching into curl privileged data.
  5. A bit easier to update jshint and buster.

@scothis I pretty much stole your code from test-support (retained your copyright and license statement). Since it's all MIT, I think that's ok, but lmk if you have any concerns.

I updated it to account for newer wd's promise chaining support, wrapped it all up in a single file (out of laziness, mainly), and made a few other mods. It might be interesting to morph this into a buster-saucelabs extension. I looked into it briefly (like 5 minutes), but couldn't find the right way to handle asynchrony in a buster extension, which is weird because buster's async support is so good otherwise.

I'll try to make some time to ask the BusterJS folks on IRC.

unscriptable commented 10 years ago

It's really nice that there's no need for an AMD config, etc. Would love to explore ways that rave could solve this. In the mean time, this looks great.

Regarding poly vs es5-shim: we really should either commit to poly or just use es5-shim and/or es6-shim. I think the answer to which we decide boils down to this question:

Is it useful for end-user devs to have a modular suite of shims or are they just using nearly 100% of them, anyways?

If it's the latter, then maybe we should deprecate poly and just use the monolithic shims?

Sorry, off-topic a bit.

briancavalier commented 10 years ago

Is it useful for end-user devs to have a modular suite of shims or are they just using nearly 100% of them, anyways?

Yeah, good question. I really don't know ... maybe we need to do a survey?

I'd use poly for these tests, but I was lazy ... the simplest thing would be just to browserify poly/es5 and add that as a script tag. I'll try it, might be easy.

jescalan commented 10 years ago

So this probably isn't that directly useful, but potentially interesting: https://ci.testling.com/

I've looked into implementing this for a couple libraries and it's still a bit buggy as far as the CI (just compared to travis it's very info-bare for debugging), but it works nicely locally.

briancavalier commented 10 years ago

I just pushed an update that uses cujojs/poly rather than es5-sham/shim. Simplifies the HTML testbed at the cost of adding an extra browserify run. I could commit the browserified poly, but it's built automatically, so who cares :)

@jenius testling seems very similar to saucelabs. Our Travis+sauce setup is nice (ignoring the recent sauce breakdown) in that it runs all our tests in node and in browsers on sauce, and will fail the Travis build if either node or browser tests fail. Github will use that same build for pull-requests as well.

Do you know what advantages testling might have over sauce? ie could we drop Travis and run both node and browser tests on testling? If so, that could be a win, since it'd remove the need for the sauce tunneling machinery that we're using to connect Travis and sauce.

jescalan commented 10 years ago

Ah I don't think testling interops with travis or runs pure node tests right now so we wouldn't get that tunneling.

briancavalier commented 10 years ago

@jenius darn, oh well. Let's keep an eye out for options, though. eg if testling adds node support, that'd be worth investigating.

Ok, these tests all pass with cujojs/poly. Barring any objections in the next couple hours, I'm gonna merge this.

@jenius this should only affect #302 in one spot: you'll be able to simplify your test file by removing the nasty boilerplate :)

jescalan commented 10 years ago

That is wonderful. I'm all for removing boilerplate :grinning:

briancavalier commented 10 years ago

Cleaned up the PR title and description since this is no longer an experiment, and rebased to a single commit for easier reading.