-
Notifications
You must be signed in to change notification settings - Fork 50
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 auth token handing in GitHub Actions #274
Fix auth token handing in GitHub Actions #274
Conversation
@ruippeixotog Thanks for the PR. Some of the (scripted) tests fail. Will try to take a look at it later this week. |
Thanks @rolandtritsch. Like in #266, I think some of the tests in sample projects are flaky and affecting this plugin (there's no reason why sbt-coveralls would cause an assertion on an Akka message to start failing). |
Hi @ruippeixotog. Merged it and made it work/pass. Do you want me to cut a release? |
That would be great, so we can finally upgrade from 1.3.2. Thanks! |
writeOpt("service_name", service.map(_.name)) | ||
coverallsAuth match { | ||
case CoverallsRepoToken(token) => | ||
gen.writeStringField("repo_token", token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it appears that service_name
was missed out in this case, which causes the upload job to fail for private repositories where the coveralls repo token is used:
Uploading to https://coveralls.io/ failed: Provided service_job_id but not service_name.
see issue #288
This should be fixed now (again) - https://github.com/scoverage/sbt-coveralls/releases/tag/v1.3.13 |
Fixes #257. See discussion in the issue for context.
The solution to support both the old behavior and the buggy one introduced in #212 ended up becoming more difficult than expected. When someone passes only
COVERALLS_REPO_TOKEN
, it's not possible in general to know if the token passed was a CI service token (old sbt-coveralls behavior for GitHub Actions) or a Coveralls token (forced by the above PR) without asking the user to change some of their settings, which would end up being another breaking change.In the end, I leveraged the fact that GitHub tokens have identifiable prefixes to support both cases.
Tested this by publishing a snapshot of sbt-coveralls, using it with PureConfig, confirming that GitHub CI jobs were successful and checking Coveralls results.