Skip to content

Handle skipped stale coverage uploads - #25

Merged
cassiomarques merged 3 commits into
mainfrom
cassiomarques/hnadle-skipped-stale-upload
Aug 6, 2026
Merged

Handle skipped stale coverage uploads#25
cassiomarques merged 3 commits into
mainfrom
cassiomarques/hnadle-skipped-stale-upload

Conversation

@cassiomarques

Copy link
Copy Markdown
Contributor

This PR changes how the coverage upload action interprets the responses received from the monolith API.

  • When 201 is returned with an upload id, this is interpreted as successful upload and the id can be used to query the status of the upload/processing (no change in behaviour)
  • When 201 is returned but an upload id is missing -> error
  • When 200 is returned and no upload id is present -> new convention, it means that this upload is for a stale commit and should be skipped. We'll mark the job as successful and move on.

The intent is to fix the bug without introducing breaking changes to the API. New installs of the upload action will no longer present the issue, whilst existing installs can be fixed by updating the version of the action (although we can't force users to do so automatically).

Copilot AI lite review requested due to automatic review settings August 5, 2026 21:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Ready to approve

The behavioral change aligns with the stated API convention and is covered by targeted tests; remaining feedback is a minor warning-message quality improvement.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Updates the upload action’s response handling to support a new API convention where a 200 response with no upload ID indicates a stale commit upload that should be skipped (job succeeds without waiting for processing), while still treating missing IDs on 201 as an error.

Changes:

  • Adjust handle_response to return an optional upload ID and treat 200 without an ID as a “skipped” upload (warning + early success).
  • Update main flow to only wait for processing when an upload ID is present.
  • Add/adjust tests and document the skipped-stale behavior in the README.
File summaries
File Description
upload_coverage.py Implements “skipped stale upload” behavior and propagates optional upload ID to processing wait logic.
test_upload_coverage.py Adds coverage for 200-without-id skip behavior and telemetry reporting expectations.
README.md Documents the new skipped-stale-commit behavior and its effect on waiting and exit code.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread upload_coverage.py Outdated

@jkbschmid jkbschmid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only nits, rest LGTM

Comment thread test_upload_coverage.py Outdated
Comment thread test_upload_coverage.py Outdated
Comment thread upload_coverage.py Outdated
Comment thread upload_coverage.py Outdated
jkbschmid
jkbschmid previously approved these changes Aug 6, 2026
Comment thread upload_coverage.py Outdated
@cassiomarques
cassiomarques force-pushed the cassiomarques/hnadle-skipped-stale-upload branch from f075c85 to e59685b Compare August 6, 2026 12:11
@cassiomarques
cassiomarques merged commit d8e3291 into main Aug 6, 2026
2 checks passed
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