From e08442ddd604403c6e6f0e7f745adcdeab2f2ccb Mon Sep 17 00:00:00 2001 From: kdmukai <934746+kdmukai@users.noreply.github.com> Date: Fri, 26 Dec 2025 14:33:20 -0600 Subject: [PATCH] Github Actions CI env var improvements --- .github/workflows/tests.yml | 1 + src/seedsigner/helpers/version.py | 38 ++++++++++++--- tests/test_version.py | 79 ++++++++++++++++++++++++++----- 3 files changed, 99 insertions(+), 19 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c1e14c97..b4165fd8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -39,6 +39,7 @@ jobs: pip install . - name: Test with pytest run: | + export PR_AUTHOR="${{ github.event.pull_request.user.login || github.actor }}" mkdir artifacts python -m pytest \ --color=yes \ diff --git a/src/seedsigner/helpers/version.py b/src/seedsigner/helpers/version.py index e9d71fc8..cc65e4fa 100644 --- a/src/seedsigner/helpers/version.py +++ b/src/seedsigner/helpers/version.py @@ -193,8 +193,10 @@ class VersionUtils: ENV_VAR__IS_SEEDSIGNER_OS_BUILDER = "SEEDSIGNER_OS_BUILDER" ENV_VAR__GITHUB_ACTIONS__IS_CI = "CI" + ENV_VAR__GITHUB_ACTIONS__HEAD_REF = "GITHUB_HEAD_REF" ENV_VAR__GITHUB_ACTIONS__REF_NAME = "GITHUB_REF_NAME" ENV_VAR__GITHUB_ACTIONS__SHA = "GITHUB_SHA" + ENV_VAR__GITHUB_ACTIONS__PR_AUTHOR = "PR_AUTHOR" ENV_VAR__GITHUB_ACTIONS__REPOSITORY_OWNER = "GITHUB_REPOSITORY_OWNER" DOT_GIT_DIR_NAME = ".git" # defined to facilitate mocking in tests VERSIONFILE__FILENAME = "version.json" @@ -264,7 +266,7 @@ class VersionUtils: return VersionUtils._get_version_fork_from_version_file() elif VersionUtils.is_github_actions_ci(): - # In Github Actions CI, try to get the version name from env vars + # In Github Actions CI, try to get the fork name from env vars. return VersionUtils._get_version_fork_from_github_actions_env_vars() else: @@ -459,16 +461,38 @@ class VersionUtils: @classmethod def _get_version_name_from_github_actions_env_vars(cls) -> str | None: - # REF_NAME will be the branch or tag name; SHA is the full commit hash - # TODO: Will REF_NAME ever be missing? - ref_name = os.getenv(cls.ENV_VAR__GITHUB_ACTIONS__REF_NAME) - sha = os.getenv(cls.ENV_VAR__GITHUB_ACTIONS__SHA) - return ref_name or (sha[:7] if sha else None) + """ + HEAD_REF: head ref or source branch. But only present for PRs. + REF_NAME: branch or tag name. But it is "/merge" for unmerged PRs (not + what we want) + SHA is the full commit hash; not expecting to ever need this fallback. + """ + for env_var in [ + cls.ENV_VAR__GITHUB_ACTIONS__HEAD_REF, + cls.ENV_VAR__GITHUB_ACTIONS__REF_NAME, + cls.ENV_VAR__GITHUB_ACTIONS__SHA + ]: + version_name = os.getenv(env_var) + if version_name: + if env_var == cls.ENV_VAR__GITHUB_ACTIONS__SHA: + # Return the short version + version_name = version_name[:7] + return version_name @classmethod def _get_version_fork_from_github_actions_env_vars(cls) -> str | None: - return os.getenv("GITHUB_REPOSITORY_OWNER") + """ + PR_AUTHOR: Set by the .github/workflows/tests.yml workflow. Should be the PR author. + REPOSITORY_OWNER: the repo owner, usually the main "SeedSigner" org. + """ + for env_var in [ + cls.ENV_VAR__GITHUB_ACTIONS__PR_AUTHOR, + cls.ENV_VAR__GITHUB_ACTIONS__REPOSITORY_OWNER + ]: + fork_name = os.getenv(env_var) + if fork_name: + return fork_name @classmethod diff --git a/tests/test_version.py b/tests/test_version.py index 9a1e40ee..8c15444b 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -365,31 +365,86 @@ class TestVersionUtils_GithubActions(VersionBaseTest): assert VersionUtils.is_github_actions_ci() is False - def test_get_version_name_from_github_actions_env_vars(self): + def test__get_version_name_from_github_actions_env_vars(self): """ - get_version_name_from_github_actions_env_vars should return the REF_NAME or SHA - env vars when set. + _get_version_name_from_github_actions_env_vars should prioritize HEAD_REF, then + REF_NAME, then SHA env vars when set. """ + test_head_ref = "head_ref" + test_ref_name = "ref_name" + test_sha = TEST__FULL_COMMIT_HASH + # Need to signal that we're in a GitHub Actions CI environment with patch.dict(os.environ, { VersionUtils.ENV_VAR__GITHUB_ACTIONS__IS_CI: "true", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: "", VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: "", VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: "", }): + # With none of the env vars set, should return None assert VersionUtils._get_version_name_from_github_actions_env_vars() is None - # REF_NAME should be passed straight through - with patch.dict(os.environ, {VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: TEST__VERSION_NAME}): - result = VersionUtils._get_version_name_from_github_actions_env_vars() - assert result == TEST__VERSION_NAME - - # Unlikely scenario: no REF_NAME but SHA is set; should return short commit hash + # HEAD_REF should be passed straight through, ignoring other vars with patch.dict(os.environ, { - VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: "", - VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: TEST__FULL_COMMIT_HASH, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: test_head_ref, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: test_ref_name, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: test_sha, }): result = VersionUtils._get_version_name_from_github_actions_env_vars() - assert result == TEST__SHORT_COMMIT_HASH[:7] + assert result == test_head_ref + + # REF_NAME is our next fallback + with patch.dict(os.environ, { + VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: test_ref_name, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: test_sha, + }): + result = VersionUtils._get_version_name_from_github_actions_env_vars() + assert result == test_ref_name + + # Unlikely scenario: no HEAD_REF nor REF_NAME but SHA is set; should return + # short commit hash + with patch.dict(os.environ, { + VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: test_sha, + }): + result = VersionUtils._get_version_name_from_github_actions_env_vars() + assert result == test_sha[:7] + + + def test__get_version_fork_from_github_actions_env_vars(self): + """ + _get_version_fork_from_github_actions_env_vars should return the PR_AUTHOR, then + fall back to REPOSITORY_OWNER. + """ + test_pr_author = "some_pr_author" + test_repo_owner = "some_repo_owner" + + # Need to signal that we're in a GitHub Actions CI environment + with patch.dict(os.environ, { + VersionUtils.ENV_VAR__GITHUB_ACTIONS__IS_CI: "true", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__PR_AUTHOR: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__REPOSITORY_OWNER: "", + }): + # With none of the env vars set, should return None + assert VersionUtils._get_version_fork_from_github_actions_env_vars() is None + + # PR_AUTHOR should be passed straight through, ignoring REPOSITORY_OWNER + with patch.dict(os.environ, { + VersionUtils.ENV_VAR__GITHUB_ACTIONS__PR_AUTHOR: test_pr_author, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__REPOSITORY_OWNER: test_repo_owner, + }): + result = VersionUtils._get_version_fork_from_github_actions_env_vars() + assert result == test_pr_author + + # REPOSITORY_OWNER is our next fallback + with patch.dict(os.environ, { + VersionUtils.ENV_VAR__GITHUB_ACTIONS__PR_AUTHOR: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__REPOSITORY_OWNER: test_repo_owner, + }): + result = VersionUtils._get_version_fork_from_github_actions_env_vars() + assert result == test_repo_owner