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.

Sign up for GitHub

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

Jump to bottom

Code coverage using cargo-travis on coveralls. #265

Merged
merged 1 commit into from
Sep 20, 2017
Merged

Code coverage using cargo-travis on coveralls. #265

merged 1 commit into from
Sep 20, 2017

Conversation

WaDelma
Copy link
Member

@WaDelma WaDelma commented Sep 17, 2017

This PR adds automatic code coverage testing using cargo-travis and coveralls.

Note: I cannot add the repo to coveralls as I am not member of the github organization.

@torkleyy
Copy link
Member

https://travis-ci.org/slide-rs/specs/jobs/276531441#L2188 :

kcov: warning: kcov: WARNING: kcov has been built without libbfd-dev (or
binutils-dev), so the --verify option will not do anything.

Should we do anything about that?

@WaDelma
Copy link
Member Author

WaDelma commented Sep 17, 2017 •
edited
Loading

I have created issue to cargo-travis for it.
Afaik kcovs --verify flag needs it to work and if it's not used it can cause segfaults.

It seems to work fine for my project so I don't know if it's actually a problem.

kvark
kvark approved these changes Sep 18, 2017
View reviewed changes
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
I added the repo to coveralls.
I suppose squashing the 2 commits wouldn't hurt before merging ;)

@WaDelma
Copy link
Member Author

WaDelma commented Sep 18, 2017

Good idea. It also tests that my change actually sends the coverage to coveralls.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 1b2a2c1 on WaDelma:coverage into ** on slide-rs:master**.

@WaDelma
Copy link
Member Author

WaDelma commented Sep 18, 2017

You should remove those coveralls messages (I don't know why they are on by default).

Also the coverage report doesn't open the files because it has empty prefix and it should be changed to src.

@torkleyy
Copy link
Member

90%? That's much more than I expected.

@torkleyy
Copy link
Member

Btw, why remove the coveralls messages?

@WaDelma
Copy link
Member Author

WaDelma commented Sep 18, 2017

It creates after every CI round. So if you push 10 commits in rapid succession it will spam 10 messages to the PR.

@torkleyy
Copy link
Member

Oh I see.

torkleyy
torkleyy approved these changes Sep 20, 2017
View reviewed changes
Copy link
Member

@torkleyy torkleyy left a comment

Choose a reason for hiding this comment

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

So.. I think we can merge this?

@torkleyy
Copy link
Member

Thank you @WaDelma!

bors r+

bors bot added a commit that referenced this pull request Sep 20, 2017
@bors
Merge #265
dc51714 
265: Code coverage using cargo-travis on coveralls. r=torkleyy a=WaDelma

This PR adds automatic code coverage testing using `cargo-travis` and `coveralls`.

Note: I cannot add the repo to coveralls as I am not member of the github organization.
@bors
Copy link
Contributor

bors bot commented Sep 20, 2017

Build succeeded

@bors bors bot merged commit 1b2a2c1 into amethyst:master Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvark kvark kvark approved these changes

@torkleyy torkleyy torkleyy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@WaDelma @torkleyy @coveralls @kvark