Skip to content

Refactor upload logic into testable Python script#7

Merged
joshhale merged 2 commits into
mainfrom
joshhale/refactor-to-python-script
May 22, 2026
Merged

Refactor upload logic into testable Python script#7
joshhale merged 2 commits into
mainfrom
joshhale/refactor-to-python-script

Conversation

@joshhale
Copy link
Copy Markdown
Collaborator

Summary

Extracts the upload logic from inline bash in action.yml into a standalone Python script (upload_coverage.py) so that the behaviour can be unit tested without running the action.

Changes

  • upload_coverage.py: Handles file validation, gzip/base64 encoding, JSON payload construction, HTTP PUT request, and error handling with Actions annotations. Uses only Python stdlib.
  • action.yml: Now a minimal harness that resolves GitHub context (merge queue skip, fork PR skip, commit SHA, ref, PR number) and delegates to the Python script.
  • test_upload_coverage.py: 11 unit tests covering every response code and input configuration (file not found, 201 success, PR-based payload, ref-based payload, missing ref+PR, 403 permissions, 403 other, 400, 500, network error, payload encoding).
  • .github/workflows/ci.yml: Runs the unit tests on push to main and on PRs.
  • .gitignore: Ignores __pycache__/.

Motivation

Per discussion in Slack, the inline bash in action.yml was getting complex and difficult to test. Moving logic into a script makes it possible to regression-test API response handling without deploying the action.

joshhale and others added 2 commits May 21, 2026 16:22
Move the upload logic (file validation, gzip/base64 encoding, payload
construction, HTTP request, error handling) out of action.yml into
upload_coverage.py. The action.yml is now a minimal harness that resolves
GitHub context (event type, commit SHA, ref, PR number) and calls the
script.

This makes the behaviour testable without running the action. The test
suite covers: file not found, successful upload (201), PR-based vs
ref-based payloads, 403 permissions errors, 403 other, 400, 500, network
errors, missing ref/PR number, and payload encoding correctness.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 21, 2026 15:45
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Comment thread .github/workflows/ci.yml
- uses: actions/checkout@v4

- name: Run unit tests
run: python3 -m unittest test_upload_coverage.py -v
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@joshhale
Copy link
Copy Markdown
Collaborator Author

I've tested this from a PR on a dsp-testing repo.
This workflow run successfully uploads coverage using the action in this PR's branch, and the coverage comment was updated as expected.

Copy link
Copy Markdown

@cassiomarques cassiomarques left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@joshhale joshhale merged commit ef71357 into main May 22, 2026
1 check passed
joshhale added a commit that referenced this pull request May 22, 2026
Resolve conflicts after PR #7 (refactor to Python script) merged to
main. The upload logic now lives in upload_coverage.py rather than
inline bash in action.yml.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

3 participants