Skip to content

Omit run URL when GITHUB_REPOSITORY or GITHUB_RUN_ID is missing - #164

Open
SirHegel wants to merge 1 commit into
buildkite:mainfrom
SirHegel:fix-github-actions-url-when-env-missing
Open

Omit run URL when GITHUB_REPOSITORY or GITHUB_RUN_ID is missing#164
SirHegel wants to merge 1 commit into
buildkite:mainfrom
SirHegel:fix-github-actions-url-when-env-missing

Conversation

@SirHegel

Copy link
Copy Markdown

Same defect as buildkite/test-collector-python#132, which was merged yesterday. I went looking for it here after fixing it there.

The problem

github_actions() builds the URL with a template literal over two environment variables that may be undefined:

"url": `https://github.com/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`,

Interpolating undefined yields the literal string "undefined", so the result is:

https://github.com/undefined/actions/runs/undefined

That is a real string rather than undefined, so JSON.stringify keeps it and it is sent as though it were a genuine URL.

Why it is reachable

ci_env() enters this branch on GITHUB_RUN_NUMBER alone:

} else if (process.env.GITHUB_RUN_NUMBER !== undefined) {
  return(this.github_actions())

Neither GITHUB_REPOSITORY nor GITHUB_RUN_ID is checked. The existing test uses github action environment variables second already walks this exact path — it sets only GITHUB_RUN_NUMBER — but it asserts only on ci, so the malformed URL goes unnoticed.

Reproduced against the current code:

process.env.GITHUB_RUN_NUMBER = '43'
new CI().env('jest').url
// 'https://github.com/undefined/actions/runs/undefined'

Why the other branches are fine

buildkite() and circleci() assign process.env values directly, so a missing variable stays undefined and JSON.stringify drops the key. This brings github_actions() in line with them.

Tests

Added a regression test that fails on the current code — Received: "https://github.com/undefined/actions/runs/undefined" — and passes with the change. Suite: 10 passed.

One thing I left alone

key has the same shape and produces undefined-43-undefined when GITHUB_ACTION and GITHUB_RUN_ATTEMPT are absent. I did not touch it because key is required and changing its format could affect how runs are grouped on your side. Happy to follow up if you would like it handled.

github_actions() builds the URL with a template literal over two environment
variables that may be undefined. Interpolating undefined yields the literal
string 'undefined', so the result is:

    https://github.com/undefined/actions/runs/undefined

That is a real string rather than undefined, so JSON.stringify keeps it and
it is sent as though it were a genuine URL.

ci_env() enters this branch on GITHUB_RUN_NUMBER alone, so the other two
variables are not guaranteed. The existing test 'uses github action
environment variables second' already exercises this exact path — it sets
only GITHUB_RUN_NUMBER — but asserts only on ci.

buildkite() and circleci() are unaffected: they assign process.env values
directly, so a missing variable stays undefined and JSON.stringify drops the
key. This brings github_actions() in line with them.

Same defect as buildkite/test-collector-python#132.

Adds a regression test that fails on the current code and passes with the
change.
@SirHegel

Copy link
Copy Markdown
Author

Small note in case it helps triage: the identical fix for the Python collector was merged as buildkite/test-collector-python#132, so the two libraries now disagree about what url holds when only one of GITHUB_REPOSITORY / GITHUB_RUN_ID is set — Python omits the key, JavaScript still sends https://github.com/undefined/actions/runs/undefined.

No rush from my side; just flagging that the divergence exists now rather than later.

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.

1 participant