Add upload diagnostics and fail-on-error option#6
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves diagnostics for the code coverage upload step by surfacing upload parameters in a collapsed log group and reporting the HTTP status code (including differentiating 201 stored vs 200 accepted-but-not-stored) along with the response body to aid troubleshooting.
Changes:
- Log key upload parameters in a grouped section to keep workflow logs readable.
- Capture and report the HTTP status code and response body from the upload API call.
- Emit warnings/notices for non-201 outcomes to make “green but no report” scenarios easier to diagnose.
Show a summary per file
| File | Description |
|---|---|
| action.yml | Adds grouped parameter logging and expands upload handling to parse/report HTTP status and response body. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 2
joshhale
force-pushed
the
joshhale/improve-upload-diagnostics
branch
from
90e0795 to
4681ec5
Compare
joshhale
changed the title
Surface the HTTP status code and response body in the action output so that users can distinguish between a true 201 (report stored) and a 200 (report silently rejected, e.g. commit not latest on branch). Parameters are logged in a collapsed group to avoid noise. Add a fail-on-error input (default: false) so that upload failures do not break existing workflows. Users who want strict behaviour can opt in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
joshhale
force-pushed
the
joshhale/improve-upload-diagnostics
branch
from
4681ec5 to
6d6f740
Compare
Testing the fail-on-error behaviourFirst, against the current version of 
Same behaviour when When 
Testing the loggingWe see the upload parameters in the log. On 200 we get a warning that no action was taken: 
No warning on 201: 
|
Strip carriage returns before splitting headers from body so that the blank-line separator is matched correctly on responses with CRLF line endings from gh api --include. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Retesting after tweaksSuccess case: 
200 case: 
|
The previous default of false silently swallowed upload errors, which is a change from the original behaviour where the action exited 1 on API errors. Default to true so workflows fail visibly on upload errors, and mention the opt-out in error messages and the README. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
|
This has been refactored after switching to a python script for the bulk of the behaviour. I've therefore done some retesting. This workflow run illustrates the case when we upload coverage for not-the-latest-commit: 
And this workflow run tests when we upload coverage for the latest commit: 
|
|
This workflow run tests the case where CQ is not enabled and we didn't use the fail-on-error setting: 
The workflow failed (as expected). |
|
This workflow run tests the case where CQ is not enabled but we set 
The workflow succeeded (as expected). |
|
@joshhale Should we already have docs at https://docs.github.com/rest/code-quality/code-coverage or is the URL shown in the workflow runs just a placeholder for now? When I visit that URL I get 404. |
Good spot. We expect to add that for GA, so I better not include it yet. https://github.com/github/code-scanning/issues/23011 |
The API response includes a documentation_url field that currently 404s since docs are not yet published. Extract just the message field for display. Added TODO comments to restore it for GA. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Adds detailed diagnostic output and a
fail-on-erroroption to the upload action, integrated into theupload_coverage.pyscript framework introduced in PR #7.Changes
upload_coverage.py::group::block before uploading.::warning::with the API message (typically "commit is not the latest on branch").::error::with permissions guidance.::error::with status code and response body.::error::indicating the API was unreachable.fail-on-error: falseoption.fail-on-errorsupport: ReadsFAIL_ON_ERRORfrom the environment. When"false", upload failures (4xx, 5xx, network errors) exit 0 instead of 1. Local configuration errors (missing file, missing ref+PR) always exit 1 regardless of this setting.action.ymlfail-on-errorinput (default:true).FAIL_ON_ERRORenv var to the Python script.test_upload_coverage.pyExpanded from 11 to 29 unit tests covering:
fail-on-error: trueexits 1 on 4xx, 5xx, and network errorsfail-on-error: falseexits 0 on 4xx, 5xx, and network errorsfail-on-error: falsestill exits 1 on config errors (missing file, missing ref+PR)fail-on-errorhintREADME.mdDocuments the
fail-on-errorinput, error handling behavior, and permissions requirements.Backward compatibility
The default behavior is unchanged: the action exits 1 on upload failures. The new
fail-on-error: falseinput provides an opt-out for workflows where coverage upload is best-effort.