Added Support for ES6 Coverage #142
Conversation
…e to Cory's badge to make it a little easier for people to see what's going on.
|
Changes Unknown when pulling a5abbb2 on dwmkerr:master into * on coryhouse:master*. |
|
This looks great! I'm at a conference so I'll need to look this over a bit later, but my only initial concern is that Isparta isn't actively maintained. I believe there are some good alternatives to consider. Thanks so much! |
|
One typo: "report is writtent to" Also, I'm confused this reference: Uncomment the line below if you have setup Coveralls as described in the FAQ.The line is already uncommented in the commit, and would have to be uncommented for coveralls to work for us, right? |
|
babel-plugin-coverage looks like it may fit the bill, although not yet particularly mature. There's a suggestion in the repo that these capabilities will come natively to istanbul. The 'uncomment' line is indeed confusing, I'll remove all of that |
|
Although would you prefer the default for people to opt-in to coverage or opt-out? |
|
Can you clarify the impact of opt-in by default? |
| @@ -17,6 +17,8 @@ | |||
| "prebuild": "npm run clean-dist && npm run build:html && npm run lint && npm run test", | |||
| "build": "babel-node tools/build.js && npm run open:dist", | |||
| "test": "mocha tools/testSetup.js \"src/**/*.spec.js\" --reporter progress", | |||
| "test:cover": "babel-node $(npm bin)/isparta cover --root src --report html node_modules/mocha/bin/_mocha -- --require ./tools/testSetup.js \"src/**/*.spec.js\" --reporter progress", | |||
| "test:cover:travis": "babel-node ./node_modules/.bin/isparta cover --root src --report lcovonly node_modules/mocha/bin/_mocha -- --require ./tools/testSetup.js \"src/**/*.spec.js\" && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js", | |||
There was a problem hiding this comment.
./node_modules/.bin is automatically added to $PATH for these scripts, babel-node isparta … should be enough. Same for test:cover
|
@dwmkerr - Can you clarify the impact of opt-in by default? |
|
@coryhouse yes indeed, basically it means that if a user runs the build in travis, it will also attempt to then send the code coverage data (if the build is successful). If the user doesn't have a coveralls account it will fail to do so, let me get a quick screenshot of what that would look like |
|
Changes Unknown when pulling 3cdd6af on dwmkerr:master into * on coryhouse:master*. |
|
OK so even without coveralls enabled the build passes successfully (it just doesn't generate a report) so I think it's good to go if you want it, I've updated the documentation just now too |
|
@dwmkerr Thanks so much! 👍 |
|
@dwmkerr FYI - I just committed a few tweaks to provide Windows support. Windows didn't like this: $(npm bin) I also removed some unnecessary explicit paths to node_modules, and fixed a few typos. Thanks again! :) |
|
@coryhouse great thanks for making the tweaks, I didn't have windows environment to test on! |
Hi Guys,
Here's a PR which adds coverage support.
Badge Support
Coverage badge added to the README.md. If the PR is accepted, the coveralls project should be changed to one Cory sets up (instructions for this are in the FAQ).
Optional integration with Coveralls
Support for the free-for-open-source coveralls tool, just check the FAQ.
Coverage commands
Just run
npm run test:coverto generate a local HTML coverage report, open it up withopen ./coverage/index.html.Would love to know your thoughts. Fixes #108. Fixes #97.
Questions
Should we keep the coveralls support on by default, or have the users opt-in?