Skip to content

fix(ci): three uploads ignored the input that already gates their consumer — so an artifact-free run is now actually artifact-free (#78) - #79

Closed
sotashimozono wants to merge 1 commit into
mainfrom
feat/78-artifact-free-mode
Closed

sotashimozono wants to merge 1 commit into
mainfrom
feat/78-artifact-free-mode

Conversation

@sotashimozono

Copy link
Copy Markdown
Member

Closes part of #78.

What is wrong

Every upload in sharded-tests.yml is continue-on-error: true after #76/#77, so a full artifact quota no longer turns a green suite red. What it still does is attempt six uploads per run — and on an account at its quota that is six failed API calls and a log full of errors nobody can act on.

Three of the six are produced for consumers that can be switched off, and the inputs that switch those consumers off do not reach the producers:

input gates the consumer gates the producer
record-timings the timings job (L874) no — L331 uploads regardless
diagnose every step that reads the records (L681, L685, L851) no — L645 and L840 upload regardless
coverage ✓ (L632, L790)
prebuild ✓ (job-level, L344)

So diagnose: false switched off the readers and left the writers running.

What this changes

Three if: guards, using the input names that already exist. No new inputs, no new concepts.

L331   if: ${{ inputs.record-timings }}
L645   if: ${{ always() && inputs.diagnose }}
L840   if: ${{ inputs.diagnose }}

always() on the per-shard records is load-bearing and kept: a failed shard is exactly when its records are worth having. It gains the switch rather than losing the semantics.

Why it matters — measured

With this, coverage: false, diagnose: false, record-timings: false uploads nothing at all.

That was worth establishing empirically, so it was. On FunctionMeasures.jl, with coverage: false alone, against an account whose artifact quota was full:

success       ci / test / Shard labels     ← previously failed here
in_progress   ci / test / shard s1 … s8    ← all eight ran

Before, every run in the fleet died at Shard labels with zero shards executedmain, feature branches and CompatHelper's own branches alike, for eighteen hours.

A suite that creates no artifacts does not care that artifact storage is full. That is a stronger position than keeping storage under the limit, because it does not depend on anyone remembering to prune, and it takes effect immediately — GitHub recalculates quota usage only every 6–12 hours, so deleting artifacts does not unblock anything today.

Scope

This is the part of #78 that needs no design decision. Not included:

…sumer (#78)

Every artifact upload here is `continue-on-error: true` (#76/#77), so a full
artifact quota no longer fails a green suite. What it still does is ATTEMPT six
uploads per run, three of which are produced for consumers that may be switched
off — and on an account at its quota that is six failed API calls a run and a
log full of errors nobody can act on.

Three inputs already exist and already gate the consuming side. None gated the
producing side:

  `record-timings`  gates the `timings` JOB (line 874) that consumes the
                    history — but not the upload that produces it (331)
  `diagnose`        gates every step that READS the records (681/685/851) —
                    but not the per-shard upload (645) nor the merged one (840)

So `diagnose: false` switched off the readers and left the writers running.
This adds the missing three; `coverage` and `prebuild` already gated theirs.

`always()` on the per-shard records is load-bearing and stays: a FAILED shard is
exactly when its records are worth having. It becomes
`always() && inputs.diagnose`, which keeps that and adds the switch.

WITH THIS, `coverage: false, diagnose: false, record-timings: false` UPLOADS
NOTHING, which is the point. Measured on FunctionMeasures with `coverage: false`
alone, against an account whose artifact quota was FULL: `Shard labels` went
green and all eight shards ran — where before every run died there with zero
shards executed. A suite that creates no artifacts does not care that artifact
storage is full. That is a stronger position than keeping storage below the
limit, because it does not depend on anyone remembering to prune.

Not the whole of #78 — coverage is still an artifact round-trip when it is on,
and sending each shard to Codecov directly would remove that too. This is the
part that needs no design decision.
@github-actions github-actions Bot added the bug Something isn't working label Aug 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📚 Docs preview: https://codes.sota-shimozono.com/TestShards.jl/previews/PR79/

(updates on each push to this PR)

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sotashimozono

Copy link
Copy Markdown
Member Author

Closing: this does not fix the failure it was opened against, and I said it did.

The three guards are correct in themselves — record-timings and diagnose gate their consumers and did not gate their producers. But they do not clear today's red, and I checked only after opening this:

Every unit ran, exactly once (L810) has no guard. diagnose gates L861 ("Diagnose the shape of the suite"), not the completeness check. So suppressing the -out upload does not suppress the step that reads it — it guarantees the records are absent while the reader still runs. With diagnose: false the run would be more certainly red, not less.

setting uploads completeness check result
defaults attempted, fail on quota runs, fails on empty parts/ red
this PR + diagnose: false none still runs, fails on empty parts/ red

So the PR body's claim that it restores the fleet was wrong. What it actually buys is that an explicit false stops the upload, which is worth having but is not a recovery path.

Reopening this as part of the real requirement in #80: the split has to work on a free plan, on self-hosted runners, with no artifacts and no hosted runner, while keeping completeness, coverage aggregation and timing history intact. A guard-only change cannot reach that.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Per-shard coverage artifacts ship 8 copies of the source tree — 85.7% of the payload is text collect already has checked out

1 participant