Migrate to Karma for testing #240
Conversation
- Use `inject-loader` for mocking `require()`'d dependencies. I tried both `karma-commonjs-plus` and `mockery` and couldn't get either of them to work after much debugging and fiddling around. `inject-loader`'s API is quite unwieldy, but it's the only one that worked. - Fixes up the flow type annotation syntax in two modules; the trailing commas were causing an error to be thrown during transformation, not sure why. - Remove all references to Jest - Add Node 0.12 back to the Travis config - I made a point to keep all the `babel` dependency versions the same, so I'm not sure why the `dist` files changed slightly. fixes #225
| @@ -23,7 +23,7 @@ module.exports = { | |||
| module: { | |||
| loaders: [ | |||
| { | |||
| test: /\.js(x|)?$/, | |||
| test: /\.js(x|)$/, | |||
There was a problem hiding this comment.
Would /\.jsx?$/ work and be cleaner?
There was a problem hiding this comment.
Yes! I was tempted to change it to that – actually I didn't want to modify this file at all to keep the diff smaller; this change is a mistake. But I'll change it to that so the syntax matches what's in karma.conf.js.
The loader here is bugging me too; in the Karma config it's babel?stage=1 and here it's babel-loader?stage=0. The former syntax is what I'm seeing recommended in documentation everywhere, do you know why this is different?
|
Wow nicely done! Please revert the dist files, we only generate them when On Thu, Jul 2, 2015 at 5:58 PM Brian Beck notifications@github.com wrote:
|
|
Appveyor's failing (https://ci.appveyor.com/project/ryan-roemer/radium/build/1.0.118) and this seems familiar. Probably related, Karma has historically been wonky with installing internal deps on practically all CI systems. We can try kicking Appveyor, but in addition we can consider a few extra hacks I have here for a Karma CI build https://github.com/FormidableLabs/full-stack-testing/blob/master/appveyor.yml |
|
OK. I've kicked Appveyor. Let's watch build https://ci.appveyor.com/project/ryan-roemer/radium/build/1.0.119 |
| expect(Prefixer.cssPrefix).toBe('-moz-'); | ||
| expect(Prefixer.jsPrefix).toBe('Moz'); | ||
| var Prefixer = require('inject!prefixer.js')({'exenv': exenv}); | ||
| expect(Prefixer.cssPrefix).to.equal('-moz-'); |
There was a problem hiding this comment.
Future comment: You'll get more informative assertion failures with something like:
expect(Prefixer).to.have.property('cssPrefix', '-moz-');... but I don't want to clog up a straight porting PR with change suggestions ;)
|
@exogen -- Great work! And nice to see I'll leave you to @ianobermiller and @alexlande for the rest of the substantive approval here... |
|
Thanks for the assertion tips, @ryan-roemer – I'll open a separate issue to track those once this PR is merged. |
| # Radium Ticket: https://github.com/FormidableLabs/radium/issues/138 | ||
| # Upstream Jest Ticket: https://github.com/facebook/jest/issues/243 | ||
| #- 0.12 | ||
| - 0.12 |
There was a problem hiding this comment.
should we add an io.js build in here as well?
- Remove Stylus file processor - Add latest stable io.js to Travis config
…test merged tests to Chai assertions.
|
Added "iojs" to the Travis config, which will pick up the latest stable release. For Appveyor, the docs say this will pick up io.js: |
|
I don't see why not. Can always change it back if there are issues.
|
|
Awesome work @exogen, thank you! |
Migrate to Karma for testing
inject-loaderfor mockingrequire()'d dependencies. I tried bothkarma-commonjs-plusandmockeryand couldn't get either of them to work after much debugging and fiddling around.inject-loader's API is quite unwieldy, but it's the only one that worked.babeldependency versions the same, so I'm not sure why thedistfiles changed slightly.fixes #225