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

Continuous integration: re-enable coveralls test coverage upload #637

Merged
merged 3 commits into from
Oct 16, 2022

Conversation

jayaddison
Copy link
Collaborator

I had disabled Coveralls during #628 (a bugfix to make our continuous integration run as we intend on multiple operating systems) because I noticed that it looked like a token value was exposed.

This pull request re-enables coveralls - I'll also check whether it makes sense to enable parallel mode at the same time.

@jayaddison
Copy link
Collaborator Author

🤔 It doesn't look like this worked - I expected to see an additional coveralls checkmark in the GitHub Actions results.

@hhursev
Copy link
Owner

hhursev commented Oct 15, 2022

Could be that we have to pass the token like in here . I'll take a look tomorrow too.

uses: AndreMiras/coveralls-python-action@develop
with:
# coveralls repo token
github-token: "SmlfzlVJy4ow55rduU7IU5GmmFCfAdGeq"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we should be able to use a GitHub encrypted secret to pass this along, and then this could become something like github-token: ${{ secrets.coveralls_repository_token }}

Copy link
Owner

Choose a reason for hiding this comment

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

Note: With the exception of GITHUB_TOKEN, secrets are not passed to the runner when a workflow is triggered from a forked repository.

which is stopping me from implementing. I'd like to see the coveralls report when builds are triggered by PRs from forked repos

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sigh, nothing is ever easy with computers :( That's an annoying limitation, makes sense to use the fixed token (I hope it's only valid for use with this repository on the Coveralls side).

@hhursev hhursev merged commit 8553024 into main Oct 16, 2022
@jayaddison jayaddison deleted the continuous-integration/re-enable-coveralls branch October 16, 2022 19:18
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.

2 participants