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

Fix environment detection for Semaphore CI #236

Merged

Conversation

browniebroke
Copy link
Contributor

@browniebroke browniebroke commented Oct 9, 2020

We use semaphore CI with several parallel job and it looks like the environment isn't detected properly currently:

  • I couldn't find an environment variable giving the pull request number, (supposed to come from PULL_REQUEST_NUMBER), however I noted SEMAPHORE_BRANCH_ID which is the same ID between builds pushed to the same branch. Example value: 3385909.
  • The SEMAPHORE_BUILD_NUMBER is the same across all workers for a given push. It sounds like it should be used as number, not as job. This starts at 1 on each branch, and increases on each push.
  • The job ID can be obtained via SEMAPHORE_CURRENT_JOB, and for a given build, each worker will have their own (e.g. 1, 2, 3, etc...). Looking at the config from the other CIs, I think it should be returned as job.

Things I'm not sure about

I don't know if these values are unique enough, or if coveralls expects some of them to be unique project wide.

If more unique values are required, I noted the following other environment variables, which might be useful instead:

  • SEMAPHORE_EXECUTABLE_UUID: a uuid value, at least unique per build, but might be per worker
  • SEMAPHORE_JOB_UUID: a uuid value, unique per job
  • SEMAPHORE_REPO_SLUG: format user/repo in Github

We use semaphore CI with several parallel job and it looks like the environment
isn't detected properly currently:

- I couldn't find an environment variable giving the pull request number,
  (supposed to come from `PULL_REQUEST_NUMBER`), however I noted
  `SEMAPHORE_BRANCH_ID` which is the same ID between builds pushed to the same
   branch. Example value: 3385909.
- The `SEMAPHORE_BUILD_NUMBER` is the same across all workers for a given push,
  and changes at each push. It sounds like it should be used as `number`, not
  as `job`. This starts at 1 on each branch, and increases on each push.
- The job ID can be obtained via `SEMAPHORE_CURRENT_JOB`, and for a given build,
  each worker will have their own (e.g. 1, 2, 3, etc...). Looking at the config
  from the other CIs, I think it should be returned as `job`.
@browniebroke browniebroke marked this pull request as ready for review October 9, 2020 16:07
@TheKevJames
Copy link
Owner

@browniebroke this looks correct to me, at least as a starting point! It might be the case that we want to replace SEMAPHORE_CURRENT_JOB with SEMAPHORE_JOB_UUID, depending on how Semaphore<->Coveralls handles parallel job aggregation, but to be perfectly honest I've never been able to predict that in advance and that always seems to fall to trial and error haha.

Thanks for the contribution! Any chance you have a Semaphore log I could check out where you've built off of this branch?

@browniebroke
Copy link
Contributor Author

Unfortunately, I can't share the Semaphore log as it's on a private project. I initially had some issues to test it, but I managed to make it work now.

It turns out the changes in this pull request aren't working as they stand, it looks like we're better off using the UUID constants actually. The SEMAPHORE_EXECUTABLE_UUID is unique per build, across workers, and using it instead of SEMAPHORE_BUILD_NUMBER produces better results.

Should be noted that these environment variables are for Semaphore classic. I looked at the docs for Sempahore 2.0 and they seem to be slightly different: https://docs.semaphoreci.com/ci-cd-environment/environment-variables/

Improve earlier fix work with both Semaphore 2.0 and classic.
Copy link
Owner

@TheKevJames TheKevJames left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for the contribution!

@TheKevJames TheKevJames merged commit ad4f8fa into TheKevJames:master Nov 19, 2020
@browniebroke browniebroke deleted the fix-semaphore-environment branch November 20, 2020 09:33
andy-maier pushed a commit to andy-maier/coveralls-python that referenced this pull request Dec 23, 2022
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