Omit run URL when GITHUB_REPOSITORY or GITHUB_RUN_ID is missing - #164
Open
SirHegel wants to merge 1 commit into
Open
Omit run URL when GITHUB_REPOSITORY or GITHUB_RUN_ID is missing#164SirHegel wants to merge 1 commit into
SirHegel wants to merge 1 commit into
Conversation
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.
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 No rush from my side; just flagging that the divergence exists now rather than later. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:Interpolating
undefinedyields the literal string"undefined", so the result is:That is a real string rather than
undefined, soJSON.stringifykeeps it and it is sent as though it were a genuine URL.Why it is reachable
ci_env()enters this branch onGITHUB_RUN_NUMBERalone:Neither
GITHUB_REPOSITORYnorGITHUB_RUN_IDis checked. The existing testuses github action environment variables secondalready walks this exact path — it sets onlyGITHUB_RUN_NUMBER— but it asserts only onci, so the malformed URL goes unnoticed.Reproduced against the current code:
Why the other branches are fine
buildkite()andcircleci()assignprocess.envvalues directly, so a missing variable staysundefinedandJSON.stringifydrops the key. This bringsgithub_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
keyhas the same shape and producesundefined-43-undefinedwhenGITHUB_ACTIONandGITHUB_RUN_ATTEMPTare absent. I did not touch it becausekeyis required and changing its format could affect how runs are grouped on your side. Happy to follow up if you would like it handled.