Skip to content

ci(coverage): stop swallowing test failures, and drop the unused codecov config - #2115

Merged
vchaindz merged 1 commit into
masterfrom
chore/fix-coverage-ci
Jul 30, 2026
Merged

ci(coverage): stop swallowing test failures, and drop the unused codecov config#2115
vchaindz merged 1 commit into
masterfrom
chore/fix-coverage-ci

Conversation

@vchaindz

Copy link
Copy Markdown
Contributor

The coverage/coveralls check is currently red on every open PR — including one that changes 4 lines of README and no Go at all. Investigating that turned up three separate issues. This PR fixes the two that live in this repository.

1. The Coverage job swallowed test failures

set -o pipefail
./ext-tools/go-acc ./... --covermode=atomic --ignore ... || true

The || true defeats the set -o pipefail on the line directly above it. A failing or panicking package was discarded, the partial coverage.txt was uploaded anyway, and the job reported success. A real test failure surfaced only as a slightly lower percentage on coveralls — indistinguishable from ordinary measurement noise.

Now the exit status is captured. The profile is still uploaded, so a failed run does not also become a reporting blind spot, and the job then fails with an explicit ::error:::

acc_status=0
./ext-tools/go-acc ./... ... || acc_status=$?
# ... filter + upload ...
if [ "$acc_status" -ne 0 ]; then
  echo "::error::go-acc exited ${acc_status}: ..."
  exit "$acc_status"
fi

In push.yml the check sits after docker rm -f minio, so the container is torn down even when the run failed.

Behaviour, verified by running the logic directly:

Case Before After
tests pass exit 0 exit 0
tests fail exit 0 (silently green) exit 1, upload still performed

2. codecov.yml was dead config

No workflow references codecov — coverage goes to coveralls via goveralls. The file sat in the repo root with an ignore: list that has never had any effect, misleading anyone who edits it. Removed.

Verification

  • both workflows still parse as YAML (5 and 10 jobs)
  • the modified run blocks pass bash -n
  • no ${{ }} interpolation introduced — the diff adds only shell-local variables, no untrusted input
  • exit-status logic exercised directly (table above)

3. Not fixed here — it is not in this repository

Two things this PR deliberately does not touch, because neither is controlled by repo config:

The coveralls status fails regardless of the result. Sampled across the open PRs:

PR coveralls says status
#2104 Coverage increased (+0.002%) failure
#2114 Coverage increased (+0.03%) failure
#2107 Coverage decreased (−0.002%) failure

Coverage going up still posts failure, so this is not a no-decrease gate. The pattern fits an absolute minimum threshold set above current coverage (~84.9%) in the coveralls.io project settings. Note the GitHub Actions Coverage job itself reports success — the red mark is an external commit status.

Measurement is nondeterministic. Consecutive master builds drift by −0.006% / −0.007%. The clearest case: the master build after #2111 reported −0.006%, and #2111 changed only docs/security/vulnerabilities/linear-fake/** — a separate Go module not in the main module’s ./..., whose true effect on coverage is exactly zero.

Consequence: any threshold at 0.01% resolution is a coin flip. Both need a change in coveralls.io project settings — raising/lowering the floor to sit below current coverage, or switching to "fail only on decrease > 0.5%" so it clears the noise band.

Follow-up worth considering

The filter chain is substring-based, not path-anchored:

cat coverage.txt | grep -v "test" | grep -v "fs" | grep -v "version" | grep -v "tools" | ...

Any path merely containing test, fs, version or tools is dropped, not just the intended packages. Left alone here on purpose: correcting it changes what is measured and would shift the reported baseline, which should be a deliberate decision rather than a side effect of a CI fix.

…cov config

The Coverage job ended its test run with `|| true`, which defeated the
`set -o pipefail` on the line directly above it. A failing or panicking package
was discarded, the partial coverage.txt was uploaded regardless, and the job
reported success. A real test failure therefore surfaced only as a slightly
lower coverage percentage on coveralls — indistinguishable from ordinary
measurement noise, and invisible in the job status.

Capture go-acc's exit status instead. The profile is still uploaded, so a failed
run does not also become a reporting blind spot, and the job then fails with an
explicit ::error:: naming the exit code. In push.yml the check is placed after
`docker rm -f minio` so the container is torn down even when the run failed.

Also removes codecov.yml. No workflow references codecov — coverage goes to
coveralls via goveralls in pull.yml and push.yml — so the file only misled
anyone editing it expecting an effect.

Verified: both workflows still parse as YAML (5 and 10 jobs); the modified run
blocks pass `bash -n`; no `${{ }}` interpolation was introduced; and the
exit-status logic was exercised directly — pass exits 0, failure exits non-zero
while still performing the upload.

Not addressed here, deliberately: coverage is measured nondeterministically
(consecutive master builds drift by ~0.006% with no coverage-relevant change),
and the external coverage/coveralls status reports failure even when coverage
increases. That status is set from the coveralls.io project settings, not from
this repository.
@coveralls

Copy link
Copy Markdown
Collaborator

Coverage Status

coverage: 84.975% (+0.02%) from 84.96% — chore/fix-coverage-ci into master

@vchaindz
vchaindz merged commit 24b5648 into master Jul 30, 2026
18 of 19 checks passed
@vchaindz
vchaindz deleted the chore/fix-coverage-ci branch July 30, 2026 08:05
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