fix(upload-coverage): an artifact that was never created is not a coverage bug - #83
Closed
sotashimozono wants to merge 2 commits into
Closed
sotashimozono wants to merge 2 commits into
sotashimozono wants to merge 2 commits into
Conversation
…erage bug
`Coverage (upload)` went red at the DOWNLOAD, before anything about coverage was
attempted:
Unable to download artifact(s): Artifact not found for name: testshards-lcov
This download is by `name:`, which hard-fails on absence — unlike the workflow's own
`pattern:` download, which succeeds with zero results. That asymmetry is why `collect`
gets as far as its completeness gate under a full quota while this job dies at step two.
An artifact that was never created is an infrastructure state, not a producer error: the
Actions storage quota is enforced at `CreateArtifact` **before any bytes are sent**, so it
refuses a 0-byte artifact exactly as it refuses a large one, and nothing here can be fixed
by failing. Losing the coverage REPORT is the correct degradation; taking the run down
with it is not.
ABSENT and EMPTY now get different treatment, because they are different claims:
absent -> a warning naming the likely cause, a step-summary line saying coverage was
not reported, and exit 0. The upload is skipped rather than sent empty.
empty -> still fatal. Something produced the artifact and put no report in it, which
is a producer bug and the message it already had.
Verifiable only in a real run — the change is three conditionals and the state it handles
is one the action cannot manufacture locally. The fleet exercises it immediately, since
every private consumer is currently in exactly this state.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
📚 Docs preview: https://codes.sota-shimozono.com/TestShards.jl/previews/PR83/ (updates on each push to this PR) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…t has not established
Three problems with the first version, found by reviewing it against what its own CI
actually exercised.
1. NOTHING TESTED THE BRANCH. Every check was green, and not one of them reached the
state the change exists for: `action-smoke` runs where the artifact IS present. I
wrote "verifiable only in a real run" in the PR body, and that was a way of not
looking — `action-smoke` already uses a local `./actions/upload-coverage`, so a PR
can exercise its own version of the action. Naming an artifact that cannot exist
reproduces the state exactly.
And the thing under test is not the message. It is whether `continue-on-error` on a
step of a COMPOSITE action is honoured at all, and whether `steps.<id>.outcome` is
readable from a later step of the same composite. If either is not true, the action
dies at the download exactly as before and the whole change is inert — with every
check still green, because nothing looked. `action-absent-artifact` is now the only
thing that would say so.
This repository is PUBLIC, so its storage is free and its own CI can never reach that
state by accident. The state has to be constructed.
2. THE MESSAGE ASSERTED A CAUSE. `outcome != 'success'` is every reason a download can
fail, not only absence, so "does not exist" was a claim the step had not established
— a network fault and a genuinely lost `-lcov` land in the same branch. It now says
the download failed, names what usually does that, and tells the reader to check
whether the run produced any artifacts at all before assuming the quota.
3. THE TRADE WAS HIDDEN BEHIND A TIDY DISTINCTION. "Absent is infrastructure, empty is a
bug" reads cleanly and is not airtight: absent can also be a bug, because the
workflow's `-lcov` upload is itself `continue-on-error`. So a real producer
regression now degrades to a warning where it used to be red. That is accepted — a
storage condition must not take down every consumer's test result — but it is written
down as an accepted cost rather than defined away.
Also `${GITHUB_STEP_SUMMARY:-}`: the block runs under `set -u`, where an unbound
variable would kill the step in the middle of degrading gracefully.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
Closing unmerged — keeping the current behaviour. What was measured here, so it does not have to be re-derived:
Consequence of keeping things as they are, stated once: |
sotashimozono
added a commit
that referenced
this pull request
Aug 25, 2026
…osed for lacking #83 measured all of this and was closed for one stated reason: this repository is public, so its own CI cannot reach the absent-artifact state by accident and the degrade path would ship untested. So the state is constructed here. WHAT CHANGES. `download-artifact` by `name:` HARD-FAILS on absence; by `pattern:` it succeeds with zero results. That asymmetry is why `collect` reaches its completeness gate under a full artifact quota while `Coverage (upload)` died at step two — same missing artifact, two outcomes. The absence is now a warning annotation and a step-summary line, and the uploader is gated on having a report as well as a token. WHY IT MATTERS NOW. Coverage is a REPORT; `Every unit ran, exactly once` is a CORRECTNESS CLAIM, and this file's own rule is that only the second may fail a suite. A private consumer whose org has exhausted Actions storage was getting a red required check for a run whose tests were all green — and it does not clear by deleting artifacts, because storage accrues in GigabyteHours. #83's closing note says so; measured downstream, 9628 expired records were purged org-wide and the newest retained artifact stayed frozen at the moment the quota filled. THE TEST. `action-degrades.yml` asks the action for an artifact no job in the run produces and requires the job to finish GREEN, plus a POSITIVE CONTROL that publishes a real lcov and takes the other branch — without which an action that did nothing at all would pass the first job, and degrading on absence would be indistinguishable from never working. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sotashimozono
added a commit
that referenced
this pull request
Aug 25, 2026
…osed for lacking (#87) #83 measured all of this and was closed for one stated reason: this repository is public, so its own CI cannot reach the absent-artifact state by accident and the degrade path would ship untested. So the state is constructed here. WHAT CHANGES. `download-artifact` by `name:` HARD-FAILS on absence; by `pattern:` it succeeds with zero results. That asymmetry is why `collect` reaches its completeness gate under a full artifact quota while `Coverage (upload)` died at step two — same missing artifact, two outcomes. The absence is now a warning annotation and a step-summary line, and the uploader is gated on having a report as well as a token. WHY IT MATTERS NOW. Coverage is a REPORT; `Every unit ran, exactly once` is a CORRECTNESS CLAIM, and this file's own rule is that only the second may fail a suite. A private consumer whose org has exhausted Actions storage was getting a red required check for a run whose tests were all green — and it does not clear by deleting artifacts, because storage accrues in GigabyteHours. #83's closing note says so; measured downstream, 9628 expired records were purged org-wide and the newest retained artifact stayed frozen at the moment the quota filled. THE TEST. `action-degrades.yml` asks the action for an artifact no job in the run produces and requires the job to finish GREEN, plus a POSITIVE CONTROL that publishes a real lcov and takes the other branch — without which an action that did nothing at all would pass the first job, and degrading on absence would be indistinguishable from never working. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One of the two jobs still failing across the fleet under a full artifact quota. The other is
collect's completeness gate, which needs the evidence transport in #82 and is not this PR.What fails today
Measured on
lab-sotashimozono/ParameterizedITensor.jlrun32238414796: the job's second step, before any coverage work.Why it is a hard failure here and not in
collectThis download names the artifact (
name:).collect's names a pattern, and apattern:download succeeds with zero results. Same missing artifacts, two different outcomes — which is whycollectreaches its completeness gate and reportsunits observed = 0while this job dies at the download.Why absence must not be fatal
The Actions storage quota is enforced at
CreateArtifact, before any bytes are sent. A 0-byte artifact is refused exactly as a large one is. So when the artifact is absent there is no producer error, nothing to retry, and nothing that failing here can fix — it converts a storage condition into a red test run.But absent and empty are different claims and now get different treatment:
lcov.infoThe
codecov-actionstep gainssteps.fetch.outcome == 'success'so it cannot be handed a report that was never fetched.Verification
Three conditionals, and the state they handle is one the action cannot manufacture locally — an absent artifact requires an account that cannot create one. It is verified by the fleet immediately: every private consumer is in exactly that state right now. Stated plainly rather than dressed up as a local test.
🤖 Generated with Claude Code