Update tests #2
Update tests #2
Conversation
|
Changes Unknown when pulling ddbb55b on jmcriffey:chore-updateTests into * on FormidableLabs:master*. |
| @@ -6,10 +6,16 @@ import sinon from "sinon"; | |||
|
|
|||
|
|
|||
| describe("fileHandler", () => { | |||
| const sandbox = sinon.sandbox.create(); | |||
There was a problem hiding this comment.
Never "do" anything outside of beforeEach|afterEach|it, just set scope. Should change to:
let sandbox;
beforeEach(() => {
sandbox = sinon.sandbox.create();
});
|
Great work! Getting there! Ping for re-review. |
|
Changes Unknown when pulling 1335fef on jmcriffey:chore-updateTests into * on FormidableLabs:master*. |
|
@ryan-roemer Comments should be addressed. |
| @@ -8,7 +8,7 @@ | |||
| "scripts": { | |||
| "build": "rimraf lib && babel src/ -d lib/", | |||
| "check-coverage": "babel-istanbul check-coverage", | |||
| "cover": "NODE_PATH=./src babel-node node_modules/.bin/babel-istanbul cover node_modules/.bin/_mocha -- --require test/main.js --recursive test", | |||
| "cover": "rimraf coverage && NODE_PATH=./src babel-node node_modules/.bin/babel-istanbul cover node_modules/.bin/_mocha -- --require test/main.js --recursive test", | |||
There was a problem hiding this comment.
NIT: Do we need ./ prefixes here and on the unit task? If not, remove because they break windows.
There was a problem hiding this comment.
Good catch. We don't need them.
|
Great work! A couple of nits in various places, after which everything LGTM without re-review (unless you want it). |
|
Changes Unknown when pulling fb7c488 on jmcriffey:chore-updateTests into * on FormidableLabs:master*. |
|
Changes Unknown when pulling c57bf80 on jmcriffey:chore-updateTests into * on FormidableLabs:master*. |
@ryan-roemer This should address the issues we talked about today. Thanks for the feedback!