Expose CI=true and GITHUB_ACTIONS env variables #215
Conversation
There was a problem hiding this comment.
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
packages/k8s/src/hooks/run-container-step.ts:36
- Add tests covering the scenario where CI is already provided in stepContainer.environmentVariables to ensure it is not overwritten by the default.
if (!('CI' in envs)) {
packages/k8s/src/hooks/prepare-job.ts:237
- Consider adding test cases to verify behavior when container.environmentVariables already includes a CI value, so the variable is preserved instead of being added again.
if (!('CI' in container['environmentVariables'])) {
packages/docker/src/dockerCommands/container.ts:52
- Include tests for scenarios where args.environmentVariables already contains a CI or GITHUB_ACTIONS value to confirm that the existing values are not overridden.
if (!('CI' in (args.environmentVariables ?? {}))) {
nikola-jokic
force-pushed
the
nikola-jokic/missing-envs
branch
from
675597f to
6bf42f0
Compare
| @@ -229,6 +229,18 @@ export function createContainerSpec( | |||
| } | |||
| } | |||
|
|
|||
| podContainer.env.push({ | |||
There was a problem hiding this comment.
What if podContainer.env['GITHUB_ACTIONS'] already exists? Should we manage it as we do with CI to prevent duplication?
| } | ||
| } | ||
|
|
||
| dockerArgs.push('-e', 'GITHUB_ACTIONS=true') |
There was a problem hiding this comment.
What if podContainer.env['GITHUB_ACTIONS'] already exists? Should we manage it as with CI to prevent duplication?
There was a problem hiding this comment.
I simply replicated the behaviror from https://github.com/actions/runner/blob/27d9c886ab9a45e0013cb462529ac85d581f8c41/src/Runner.Worker/Container/DockerCommandManager.cs#L146
| @@ -62,6 +62,28 @@ describe('Prepare job', () => { | |||
| ).resolves.not.toThrow() | |||
| }) | |||
|
|
|||
| it('should prepare job with envs CI and GITHUB_ACTIONS', async () => { | |||
There was a problem hiding this comment.
Should we include a test to confirm the outcome if either the GITHUB_ACTIONS or the CI environment variable already exists in the job definition?
nikola-jokic
force-pushed
the
nikola-jokic/missing-envs
branch
from
5f4668b to
118262c
Compare
nikola-jokic
force-pushed
the
nikola-jokic/missing-envs
branch
from
118262c to
92b6492
Compare
Fixes #211