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

Env Var COVERALLS_SERVICE_JOB_NUMBER maps to wrong config variable #303

Closed
ashleysommer opened this issue Jun 10, 2021 · 4 comments · Fixed by #314
Closed

Env Var COVERALLS_SERVICE_JOB_NUMBER maps to wrong config variable #303

ashleysommer opened this issue Jun 10, 2021 · 4 comments · Fixed by #314

Comments

@ashleysommer
Copy link

ashleysommer commented Jun 10, 2021

SERVICE_JOB_NUMBER and SERVICE_NUMBER are different things.

  • SERVICE_NUMBER should be the service build id, eg build #1938
  • SERVICE_JOB_NUMBER should be the the build step, eg step #2
  • SERVICE_JOB_ID should be the name of the build step, eg "pytest with coverage"

The CI-specific config handlers get the job_id right, but the code that processes environment vars seems to get it a bit confused, and doesn't provide a way to set SERVICE_JOB_ID.

number = os.environ.get('COVERALLS_SERVICE_JOB_NUMBER')
if number:
self.config['service_number'] = number

Should be:

    number = os.environ.get('COVERALLS_SERVICE_NUMBER')
    if number:
        self.config['service_number'] = number

    job_number = os.environ.get('COVERALLS_SERVICE_JOB_NUMBER')
    if job_number:
        self.config['service_job_number'] = job_number

    job_id = os.environ.get('COVERALLS_SERVICE_JOB_ID')
    if job_id:
        self.config['service_job_id'] = job_id
@ashleysommer
Copy link
Author

Kind-of related to #300

@ashleysommer
Copy link
Author

I'm basing this on the descirptions of the ENV Vars documented in the node-coveralls library: https://github.com/nickmerwin/node-coveralls
Looking at the coveralls API docs, I see that service_job_number doesn't exist in the api docs, however libraries do use it:
https://github.com/nickmerwin/node-coveralls/blob/21f7b1da017e1b2386b4354a0d8345937294fd26/lib/getOptions.js#L97

@ashleysommer
Copy link
Author

ashleysommer commented Jun 10, 2021

Found the related issue here:
lemurheavy/coveralls-public#1450
service_job_number is an undocumented api field, but it does exist.

service_job_number - is an optional non-unique job identifier; it's available if you want to identify each job yourself, and don't want them auto-numbered, or if you expect the jobs to run out-of-sequence and want to make sure they can be identified across builds.
service_build_url - also optional; you can pass the return link to the job at your CI; it's otherwise inferred for the officially supported CIs.
service_branch - this is an optional way of setting the branch; it should be the same as git[:branch] in the git hash, but if for some reason the git repo hasn't been checked out at CI build time, this is a simpler way of setting it than building up the git hash.

That matches up with my description in the initial comment.

TheKevJames added a commit that referenced this issue Jul 20, 2021
Fixes the mapping between default (eg. `COVERALLS_*`) env vars and the
equivalent config values, which had some mismatches for job
identification cases.

Fixes #303
@TheKevJames
Copy link
Owner

@ashleysommer thanks for the detailed issue! Definitely looks like we've made a mistake here and set some incorrect values; I've opened a PR (#314) with the fixes you've recommended -- if you could give it a glance over to make sure I've caught all your comments, that'd be much appreciated!

TheKevJames added a commit that referenced this issue Nov 3, 2021
Fixes the mapping between default (eg. `COVERALLS_*`) env vars and the
equivalent config values, which had some mismatches for job
identification cases.

Fixes #303
andy-maier pushed a commit to andy-maier/coveralls-python that referenced this issue Dec 23, 2022
Fixes the mapping between default (eg. `COVERALLS_*`) env vars and the
equivalent config values, which had some mismatches for job
identification cases.

Fixes TheKevJames#303
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 a pull request may close this issue.

2 participants