-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 #1193
Add code coverage #1193
Conversation
Wow, this is pretty cool, Ananyo. Will it outright reject poorly-formatted code, or will it provide helpful feedback when run (and is there any way we can ensure it will)? Thanks! |
Well if we set a benchmark to the coverage then it will fail the build if test coverage falls below that. Otherwise it helps to show useful information like test coverage and linting errors, security warnings in the coverage link. I will add the badges in readme. I am using coveralls for the test coverage and code climate for other useful information. |
b1145eb
to
64deeec
Compare
So for now it just shows helpful notes on formatting, but we haven't yet set benchmarks? So, i see this:
(here) Does that mean they didn't run in Travis? |
This is not ready yet. The CI isn't sending the coverage data as you can see in the line
Looking for possible solutions in these links: lemurheavy/coveralls-public#190, lemurheavy/coveralls-public#782 |
454eef7
to
24138d5
Compare
2fbd8ba
to
f5b854e
Compare
@jywarren This is good to go. You can get coverage and other issues from the badges included in readme: https://github.com/ananyo2012/plots2/tree/coverage#publiclaborg |
The issues section in codeclimate shows various linting and security issues with the code. The test coverage is shown using coveralls. I used coveralls for test coverage since codeclimate requires admin access to get the repo_token. Coverall only requires a write access to the repo. Sorry for too many builds occuring in the CI for the same PR as I was getting a tough time to figure out what was causing coveralls not sending reports. I finally figured this was because we were using docker for running tests which set the |
Oh cool, interesting, i'll check it out now! |
Oh neat:
85% is higher than i'd expected! |
Thanks, Ananyo, fantastic work!! 🎉 |
It would be really cool if we hit 100% code coverage and enforce it once we reach it ;) |
agreed!
…On Tue, Jan 24, 2017 at 11:12 AM, Kaisar Arkhan (Yuki) < ***@***.***> wrote:
It would be really cool if we target 100% code coverage and enforce it
once we reach it ;)
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#1193 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ2j05hsOXG85Cwk0q4yn4jAH-sLCks5rViL7gaJpZM4LntSU>
.
|
Closes #1191
Make sure these boxes are checked before your pull request is ready to be reviewed and merged. Thanks!
rake test:all
schema.rb.example
has been updated if any database migrations were addedPlease be sure you've reviewed our contribution guidelines at https://publiclab.org/wiki/contributing-to-public-lab-software
We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays. Please alert developers on plots-dev@googlegroups.com when your request is ready or if you need assistance.
Thanks!