Build kcov on travis and send reports to coveralls. #167
Conversation
| @@ -121,6 +121,11 @@ def format_entries(): | |||
| %(sources)s""" | |||
|
|
|||
| def format_entry(os, target, compiler, rust, mode): | |||
| if os == "linux" and compiler == "gcc-5" and rust == "nightly" and mode == "DEBUG": | |||
There was a problem hiding this comment.
Please write this as kcov = os == "linux" and compiler == "gcc-5" ... (i.e. without the if...else).
Why only Nightly? If there's a technical reason, please explain it in a comment. I guess I would prefer to (also) do it on stable, since stable is what people will be using (in the long term).
Similarly, please write a comment explaining why you picked gcc-5 and debug mode.
RE debug mode: Is the issue that kcov requires debug symbols to work, and we don't have debug symbols in release builds? If so, perhaps we should just change, as we should have debug symbols for release builds anyway.
There was a problem hiding this comment.
I picked nightly because passing -C link-dead-code to rustc using RUSTFLAGS currently only works on beta and nightly. I can change to stable and it will start working when the next release happens. Until then coverage tracing won't be as accurate.
I picked gcc-5 as an example. I wanted to run kcov on just one build. Should I do it on all of them?
The debug symbols are needed for coverage tracking. Should I change the release builds to add them too on this pr?
There was a problem hiding this comment.
I picked nightly because passing -C link-dead-code to rustc using RUSTFLAGS currently only works on beta and nightly. I can change to stable and it will start working when the next release happens. Until then coverage tracing won't be as accurate.
It's good to leave it as you have it. But, please add a comment with the explanation of why.
I picked gcc-5 as an example. I wanted to run kcov on just one build. Should I do it on all of them?
I picked gcc-5 as an example. I wanted to run kcov on just one build. Should I do it on all of them?
No, it is fine to just use gcc-5. But, please add a comment saying that it is restricted to one build for efficiency reasons only, and gcc-5 was chosen somewhat arbitrarily.
The debug symbols are needed for coverage tracking.
Just add a comment explaining that coverage is run on debug builds because kcov requires debug symbols.
Should I change the release builds to add them too on this pr?
It would be awesome if you did this, but perhaps a separate PR?
|
This looks awesome! Thanks for this! |
|
Is there anything I can do to help with this? |
…d gcc 5. I agree to license my contributions to each file under the terms given at the top of each file I changed.
|
Sorry I got busier than I thought I would. I incorporated the feedback and rebased my commits. I hope that's alright. |
I agree to license my contributions to each file under the terms given at the top of each file I changed.
Travis does this automatically and we need them in case we're running kcov.
|
Thanks a ton for doing this! I committed these changes in 9e3fc98 (basically your first two commits merged together) and 0b5e21b. I tweaked some of the comments and formatting but for the most part these commits are what you submitted for review. In separate commits, I also made these changes: 7e2aa42, e571668, and c447603. If you have time, please let me know if I screwed them up. Anyway, this is really exciting because, just looking at the coverage reports already, I can see several areas we can improve it all immediately. Very cool! |
|
Cool! Your changes don't seem to break anything. I saw you set coveralls and coverage changed when you generated debug symbols for assembly files. I'm glad I can help ring in some way. |
Only tracking coverage for X86 and X86_64 Linux as discussed on #132.