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

add coverage to circleci #610

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

Conversation

davidhyman
Copy link
Contributor

probably don't merge just yet, let's see what circleci does, maybe add coveralls

davidhyman and others added 3 commits January 11, 2018 15:59
@davidhyman
Copy link
Contributor Author

Please could someone with admin rights on the circle build configure the following environment parameter:
COVERALLS_SERVICE_NAME=repo token

@screamerbg
Copy link
Contributor

screamerbg commented Jan 12, 2018

Can you push a fake commit? Restarting the job doesn't take into account new environment variables.

@screamerbg
Copy link
Contributor

screamerbg commented Jan 12, 2018

Still fails even though the environment variable. I'm looking at it as we speak.

On a side note, why not move coveralls in after_success section, so CI builds don't fail due to coveralls service issues? E.g.

after_success:
  - coveralls

@davidhyman
Copy link
Contributor Author

yes lets do that once it's working 👍

@screamerbg
Copy link
Contributor

@davidhyman CircleCI is reporting issue with:

$ coveralls

Submitting coverage to coveralls.io...
Coverage submitted!
Couldn't find a repository matching this job.
'url'

coveralls returned exit code 2

Before the error CircleCI reports Suppressing export of environment variables COVERALLS_REPO_TOKEN fork PR builds

Any idea?

@screamerbg
Copy link
Contributor

screamerbg commented Jan 12, 2018

Apparently there's an option in CircleCI which allows env vars to be exposed to the container. It's generally considered unsafe and turned off by default. I turned it on to check whether it would build successfully, though this doesn't seems like the way to go.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10de503 on davidhyman:d/coverage_clean into ** on ARMmbed:master**.

@davidhyman
Copy link
Contributor Author

Sorry for the delay; yes absolutely we should not use that feature! ⚠️

The risk is that any fork PR built could include code that exports the entire environment which, if it contains tokens for git or whatever, would be a Bad Thing. Hopefully there's no private keys there used for signing stuff, for example.

If you create a new native branch and move the PR onto it, that should work.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 10de503 on davidhyman:d/coverage_clean into ** on ARMmbed:master**.

@screamerbg
Copy link
Contributor

@davidhyman How can we enable this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants