Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code coverage using NYC and integrate with codecov #213

Merged
merged 3 commits into from Jan 21, 2017
Merged

Add code coverage using NYC and integrate with codecov #213

merged 3 commits into from Jan 21, 2017

Conversation

addityasingh
Copy link
Contributor

  • Closes Add code coverage using NYC and integrate with codecov #212
  • Add code coverage using NYC
  • Separate out system tests from the unit test
  • Add scripts:
    • npm run cover: To check coverage locally
    • npm run system-test: To run system test
    • npm run check-coverage: To check and ensure that coverage is above 80%
    • npm run report-coverage: To create .lcov based code coverage report
    • npm run codecov: To upload code coverage report to codecov
  • Update .travis.yml to add the scripts to be executed at right build life-cycle events

@codecov-io
Copy link

Current coverage is 99.65% (diff: 100%)

No coverage report found for master at 20e3925.

Powered by Codecov. Last update 20e3925...ee41d2a

@addityasingh
Copy link
Contributor Author

addityasingh commented Jan 20, 2017

@nene @abouthiroppi Guys the code coverage report is available here: https://codecov.io/gh/lebab/lebab. If you add codecov app to list of authrised applications for this repo, we will start getting comments for diff of code coverage as well.

Copy link

@hiroppy hiroppy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Rubber-stamp LGTM.

@nene
Copy link
Collaborator

nene commented Jan 20, 2017

This looks great. I'll take another look at this later today.

I think I'll need to fiddle a bit with dependencies between NPM scripts. Like the test script doesn't actually need the prepublish script, only the system-test does, but we should run prepublish whenever system-test is run.

It's also unclear to me that codecov task actually performs an upload. I think we need a better name to communicate it's intent.

@addityasingh
Copy link
Contributor Author

Cool. Add your review comments and I will fix them based on your suggestions. I generally go with codecov because that makes it clear that the script is related to codecov. But any better suggestions can be applied as well

@nene
Copy link
Collaborator

nene commented Jan 20, 2017 via email

@addityasingh
Copy link
Contributor Author

@nene Based on your comments, I think the below would make sense even for someone who hasn't used codecov. Let me know what do you think?

  • cover -> check-coverage-dev
  • check-coverage -> ensure-coverage
  • codecov -> upload-coverage

@nene nene merged commit ee41d2a into lebab:master Jan 21, 2017
@nene
Copy link
Collaborator

nene commented Jan 21, 2017

Tweaked it quite a bit. Performed some renames and reorganization of package.json scripts. All merged now, with coverage reports nicely running :)

Thanks once more!

@addityasingh
Copy link
Contributor Author

Thanks a lot for taking care of the naming suggestions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants