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

Fix coveralls integration #66

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

johanneskoester
Copy link

@johanneskoester johanneskoester commented Jan 19, 2017

This PR provides a fix for the broken coveralls integration.

  1. It also looks in stderr for the output of cargo. Otherwise, the test binary won't be found by the regex.
  2. It pins kcov to a specific version, in order to avoid unexpected changes in the behavior of kcov.
  3. It creates the kcov output directory before execution. Otherwise, kcov would exit with an error.

This fixes issue #58.
Note that this PR subsumes #64, #61, and #55.

@autohuonw
Copy link
Collaborator

Thanks for the pull request, and welcome! You should hear from @huonw (or someone else) soon.

@coveralls
Copy link

coveralls commented Jan 19, 2017

Coverage Status

Coverage increased (+9.6%) to 88.889% when pulling 12b95ad on johanneskoester:master into af51893 on huonw:master.

6 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+9.6%) to 88.889% when pulling 12b95ad on johanneskoester:master into af51893 on huonw:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+9.6%) to 88.889% when pulling 12b95ad on johanneskoester:master into af51893 on huonw:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+9.6%) to 88.889% when pulling 12b95ad on johanneskoester:master into af51893 on huonw:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+9.6%) to 88.889% when pulling 12b95ad on johanneskoester:master into af51893 on huonw:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+9.6%) to 88.889% when pulling 12b95ad on johanneskoester:master into af51893 on huonw:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+9.6%) to 88.889% when pulling 12b95ad on johanneskoester:master into af51893 on huonw:master.

@johanneskoester
Copy link
Author

johanneskoester commented Jan 19, 2017

Some test cases fail, but they don't look related to my changes. In fact, they also fail with the master branch without my changes.

@SimonKagstrom
Copy link

SimonKagstrom commented Jan 19, 2017

Out of interest, what options are passed to kcov? I don't remember removing any command line option since v31, there are just a few added ones (speaking as the kcov developer). It would be great to just see the entire kcov command line from a failing run.

There are improvements and bug fixes in kcov since v31, so it would be unfortunate to be stuck with that. That said, I agree fixing a version is good - the kcov HEAD is sometimes broken for obvious reasons.

@johanneskoester
Copy link
Author

This PR uses v33 already. It turned out that the recent changes where not the cause of the problem. It was rather a combination of changes in rust and bugs in the codebase of travis-cargo.

@gnzlbg
Copy link

gnzlbg commented Feb 24, 2017

Any progress on this, @huonw ?

@gnzlbg
Copy link

gnzlbg commented Feb 24, 2017

I've posted this on internals: https://users.rust-lang.org/t/travis-cargo-status-and-future/9621

@fabricedesre
Copy link

Is anyone trying to push that to the finish line?

@johanneskoester
Copy link
Author

I have moved to this one: https://github.com/roblabla/cargo-travis/
It works flawlessly and is pure rust.

@fabricedesre
Copy link

Thanks @johanneskoester !

@janhohenheim
Copy link

@huonw any plans on merging this and the other PRs? Even though this repo is no longer actively maintained, it would be nice if the community made fixes found a way into the main branch.

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.

8 participants