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

Update TRAVIS CI to add coveralls #287

Merged
merged 1 commit into from
Dec 7, 2016
Merged

Conversation

cdrage
Copy link
Member

@cdrage cdrage commented Nov 16, 2016

Updates travis to enable coveralls support.

Fixes #281

@cdrage cdrage mentioned this pull request Nov 16, 2016
@sebgoa
Copy link
Contributor

sebgoa commented Nov 29, 2016

I have access to coveralls via incubator. I will test this today.

Copy link
Contributor

@sebgoa sebgoa left a comment

Choose a reason for hiding this comment

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

that should be good.

You can add the badge to the README:

[![Coverage Status](https://coveralls.io/repos/github/kubernetes-incubator/kompose/badge.svg?branch=master)](https://coveralls.io/github/kubernetes-incubator/kompose?branch=master)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 29, 2016
@cdrage
Copy link
Member Author

cdrage commented Nov 29, 2016

@sebgoa done. now let's see if these tests pass :)

Updates travis to enable coveralls support.

Fixes kubernetes#281
@cdrage
Copy link
Member Author

cdrage commented Nov 30, 2016

Currently this isn't working for some odd-reason (timing out). I'll be looking at an alternative fix.

@sebgoa sebgoa merged commit 0b5d167 into kubernetes:master Dec 7, 2016
@sebgoa
Copy link
Contributor

sebgoa commented Dec 7, 2016

ok, I am going to merge it and fix it

@kadel
Copy link
Member

kadel commented Dec 7, 2016

Why merge it if its not working?
It looks like it is missing token https://coveralls.zendesk.com/hc/en-us/articles/201342809-Go

@sebgoa
Copy link
Contributor

sebgoa commented Dec 7, 2016

because I was tired of seeing this PR pending and seeing our code coverage on the front page will be good motivation to improve unit tests.

afaik, you don't need a token for public repos...

but yeah it is failing.

@kadel
Copy link
Member

kadel commented Dec 7, 2016

To be honestly I don't see reason why it couldn't be fixed in this PR.

Now front page shows no coverage and failed tests.

I've done some digging around, and it is timeouting because it runs all unit test again for every package as separate go test run.
like this:

for package in all_packages:
   go test package

it takes a long time to start go test and travis-ci jobs has 15min timeout.

Root cause for this is that for some reason go test can't collect coverage profile for multiple packages.
So you have to run it for every package again :-(

We could run tests and collect all coverageprofiles into one file and than call goveralls with -coverprofile to only submit results to coveralls.io as described here https://gist.github.com/rjeczalik/6f01430e8554bf59b88e

But it still requires to run go test for every package separately.

@kadel
Copy link
Member

kadel commented Dec 7, 2016

this is how kubernetes is dealing with this https://github.com/kubernetes/kubernetes/blob/master/hack/make-rules/test.sh#L228

@cdrage cdrage deleted the add-coveralls branch March 30, 2017 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants