Skip to content

[BUG] Unit Test workflow cannot pass on pull requests from forks #316

Description

@dulcetberg

Describe the bug

The Unit Test workflow cannot pass on a pull request opened from a fork. Every test errors during collection, before any of them run, so a fork PR always shows a red Unit Test regardless of what it changes.

GitHub does not pass repository secrets to pull_request runs from forks. Unit-Test.yml sources three boolean feature flags from secrets:

ALLOW_BIND_ZIP_FILTER: ${{ secrets.ALLOW_BIND_ZIP_FILTER }}
ENABLE_HDX_EXPORTS: ${{ secrets.ENABLE_HDX_EXPORTS }}
ENABLE_POLYGON_STATISTICS_ENDPOINTS: ${{ secrets.ENABLE_POLYGON_STATISTICS_ENDPOINTS }}

On a fork PR those are set to empty strings rather than left unset. get_bool_env_var in src/config.py then fails:

def get_bool_env_var(key, default=False):
    value = os.environ.get(key, default)   # key exists but is "", so default is skipped
    return bool(strtobool(str(value)))     # strtobool("") raises

Resulting in:

src/config.py:195: in <module>
    ALLOW_BIND_ZIP_FILTER = get_bool_env_var(
src/config.py:35: in get_bool_env_var
    return bool(strtobool(str(value)))
E   ValueError: invalid truth value ''

ERROR tests/test_API.py - ValueError: invalid truth value ''
ERROR tests/test_app.py - ValueError: invalid truth value ''

To Reproduce

  1. Open a pull request from a fork against develop.
  2. Wait for the Unit Test workflow to be approved and run.
  3. Both test modules error at import. Exit code 2, zero tests executed.

Seen on #315: https://github.com/hotosm/raw-data-api/actions/runs/35638149752

Expected behavior

The suite runs on fork PRs, with feature-flagged tests skipping or defaulting rather than the whole collection failing.

Why this hasn't been noticed

Pushes to develop are not affected, because pushes to the upstream repo do get secrets. develop currently reaches the tests and reports 2 failed, 37 passed. The difference in behaviour only shows up on fork PRs.

The contrast within the workflow points at the same cause. ENABLE_TILES and SETUP_INITIAL_TABLES are not set in Unit-Test.yml at all, so their fallbacks apply and they work fine on forks. ENABLE_CUSTOM_EXPORTS is hardcoded to True and also works. Only the three sourced from secrets break.

Possible fixes

  1. Treat an empty value as unset in get_bool_env_var, for example value = os.environ.get(key) or default. This also helps anyone running with an empty env var locally.
  2. Hardcode the non-sensitive booleans in Unit-Test.yml the way ENABLE_CUSTOM_EXPORTS already is. None of the three look like they need to be secret.

Happy to open a PR for either, whichever you prefer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions