Mark superseded commits honestly instead of skipped (closes #152)
Some checks failed
check / check (push) Superseded by a newer commit; never tested
Some checks failed
check / check (push) Superseded by a newer commit; never tested
Gitea records a cancelled run as failure, and the #119 repair rewrote that to skipped. Gitea's Combine() folds skipped into success, so a commit nothing ever tested reported a combined green — observed on three commits on next, including the very change a prior integration review had failed a PR for. Superseded commits are now marked failure with an honest description, so never-tested no longer reads as passed and git bisect archaeology can tell "passed", "failed" and "never ran" apart. Option 1, re-running the superseded commit, was verified unreachable for automation on Gitea 1.25.4: no rerun endpoint, dispatches takes a ref not a SHA and lands under a different context, and CancelPreviousJobs is unconditional. The rewrite moves out of the workflow into script/ci-mark-superseded so the tested artifact is the shipped one, and every failure path in it is loud: an unparseable or empty ANCESTOR_LIMIT, an unreadable ancestor status, and a shallow clone all abort rather than exiting 0 having marked nothing. Each has a regression test. The status context is derived rather than hardcoded, which also closes #147 item 2; item 1 remains open. Independently reviewed four times. The final reviewer confirmed the shallow-clone test is genuinely shallow — a file:// URL is load-bearing, since git silently ignores --depth on a local path — and that deleting the guard fails that one test out of 331 and cannot pass for the wrong reason. They also reproduced deterministically that go test's cache serves a stale PASS after a script-only edit, which internal/ciscript's doc.go now records.
This commit was merged in pull request #161.
This commit is contained in:
@@ -13,39 +13,19 @@ jobs:
|
|||||||
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
|
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
|
||||||
with:
|
with:
|
||||||
# The fingerprint step below needs history to find the last commit
|
# The fingerprint step below needs history to find the last commit
|
||||||
# that touched the Docker build context.
|
# that touched the Docker build context, and the superseded-status
|
||||||
|
# step needs it to walk ancestors (it aborts on a shallow clone).
|
||||||
fetch-depth: 0
|
fetch-depth: 0
|
||||||
|
|
||||||
- name: Neutralize superseded run statuses
|
- name: Mark superseded run statuses
|
||||||
# Gitea cancels the in-flight run when another commit is pushed to the
|
# Gitea cancels the in-flight run when another commit is pushed to the
|
||||||
# same branch and records the cancellation as `failure`, so a commit
|
# same branch and records the cancellation as `failure`, so a commit
|
||||||
# that was never tested reads red. The cancellation is unconditional
|
# that was never tested reads as a test result. The script rewrites
|
||||||
# server-side for push events and cannot be disabled from a workflow
|
# those statuses to say what happened. See its header for why the
|
||||||
# file, so the superseding run rewrites those statuses to `skipped`.
|
# state stays `failure` and not `skipped`.
|
||||||
# Only the exact cancellation status is touched; a real failure is
|
|
||||||
# left alone.
|
|
||||||
env:
|
env:
|
||||||
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
|
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
|
||||||
run: |
|
run: script/ci-mark-superseded
|
||||||
set -eu
|
|
||||||
api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
|
|
||||||
ctx='check / check (push)'
|
|
||||||
for sha in $(git rev-list --max-count=20 "${GITHUB_SHA}^" || true); do
|
|
||||||
latest="$(curl -sf "${api}/commits/${sha}/status" | jq -r \
|
|
||||||
--arg c "$ctx" \
|
|
||||||
'[.statuses[] | select(.context == $c)][0] // empty
|
|
||||||
| "\(.status)|\(.description)"')" || continue
|
|
||||||
[ "$latest" = 'failure|Has been cancelled' ] || continue
|
|
||||||
curl -sf -X POST "${api}/statuses/${sha}" \
|
|
||||||
-H "Authorization: token ${GITEA_TOKEN}" \
|
|
||||||
-H 'Content-Type: application/json' \
|
|
||||||
-d "$(jq -nc --arg c "$ctx" '{
|
|
||||||
context: $c,
|
|
||||||
state: "skipped",
|
|
||||||
description: "Superseded by a newer commit; never tested"
|
|
||||||
}')" >/dev/null
|
|
||||||
echo "neutralized superseded status on ${sha}"
|
|
||||||
done
|
|
||||||
|
|
||||||
- name: Fingerprint the build context
|
- name: Fingerprint the build context
|
||||||
# `.dockerignore` keeps docs out of the build context, so a docs-only
|
# `.dockerignore` keeps docs out of the build context, so a docs-only
|
||||||
|
|||||||
@@ -32,7 +32,9 @@ FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a349228
|
|||||||
# Depend on lint stage passing
|
# Depend on lint stage passing
|
||||||
COPY --from=lint /src/go.sum /dev/null
|
COPY --from=lint /src/go.sum /dev/null
|
||||||
|
|
||||||
RUN apt-get update && apt-get install -y --no-install-recommends make curl ca-certificates && rm -rf /var/lib/apt/lists/*
|
# jq is a runtime dependency of script/ci-mark-superseded, which the test
|
||||||
|
# suite executes.
|
||||||
|
RUN apt-get update && apt-get install -y --no-install-recommends make curl ca-certificates jq && rm -rf /var/lib/apt/lists/*
|
||||||
|
|
||||||
WORKDIR /build
|
WORKDIR /build
|
||||||
|
|
||||||
|
|||||||
33
README.md
33
README.md
@@ -282,6 +282,8 @@ are inline commands with no script behind them. We provide:
|
|||||||
- `script/docker` — build the Docker image tagged via `script/projectname`
|
- `script/docker` — build the Docker image tagged via `script/projectname`
|
||||||
- `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile
|
- `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile
|
||||||
runs the checks, so a green build implies a green repo)
|
runs the checks, so a green build implies a green repo)
|
||||||
|
- `script/ci-mark-superseded` — CI helper: mark the commits whose run a
|
||||||
|
newer push cancelled (see [CI gate honesty](#ci-gate-honesty))
|
||||||
- `script/precommit` — pre-commit checks (`go mod tidy` guard, then
|
- `script/precommit` — pre-commit checks (`go mod tidy` guard, then
|
||||||
`script/check`)
|
`script/check`)
|
||||||
- `script/install-precommit` — install the git pre-commit hook that
|
- `script/install-precommit` — install the git pre-commit hook that
|
||||||
@@ -1445,11 +1447,32 @@ way.
|
|||||||
A separate workflow step, run before the fingerprint is written, covers
|
A separate workflow step, run before the fingerprint is written, covers
|
||||||
a second way the gate lied: Gitea cancels an in-flight run when a newer
|
a second way the gate lied: Gitea cancels an in-flight run when a newer
|
||||||
commit lands on the same branch and records that cancellation as a
|
commit lands on the same branch and records that cancellation as a
|
||||||
`failure` status, marking a commit red that was never tested.
|
`failure` status, so a commit nothing ever tested reads as a test
|
||||||
Cancellation is unconditional server-side for
|
result. Cancellation is unconditional server-side for push events, so
|
||||||
push events, so the superseding run rewrites the exact
|
the superseding run calls `script/ci-mark-superseded`, which rewrites
|
||||||
`Has been cancelled` status to `skipped`. Genuine failures are never
|
that exact status to `failure` /
|
||||||
touched.
|
`Superseded by a newer commit; never tested`.
|
||||||
|
|
||||||
|
The state stays `failure` on purpose: Gitea's combined status folds
|
||||||
|
`skipped` into `success`, so marking a never-tested commit `skipped`
|
||||||
|
made the status API report green for it, indistinguishable from a commit
|
||||||
|
that passed. Reading a commit's status on this repo therefore goes:
|
||||||
|
|
||||||
|
- `success` / `Successful in ...` — the checks ran and passed.
|
||||||
|
- `failure` / `Failing after ...` — the checks ran and failed.
|
||||||
|
- `failure` / `Superseded by a newer commit; never tested` — the run was
|
||||||
|
cancelled, by a newer push or by hand, and nothing was verified about
|
||||||
|
this commit. Test the commit itself before concluding anything about
|
||||||
|
it.
|
||||||
|
|
||||||
|
Genuine failures and successes are never touched, and no status is left
|
||||||
|
`pending`, which would block the commit indefinitely. The step derives
|
||||||
|
its context string from the workflow name, the job **id** and the event.
|
||||||
|
That is deliberately not byte-identical to Gitea's own rule, which uses
|
||||||
|
the job's display `name:` where the runner exports the id, so giving the
|
||||||
|
job a `name:` — or renaming the workflow — makes the derived context
|
||||||
|
stop matching. The step fails loudly when no status on the commit
|
||||||
|
carries that context, so no rename can silently disable the rewrite.
|
||||||
|
|
||||||
## TODO
|
## TODO
|
||||||
|
|
||||||
|
|||||||
2
go.mod
2
go.mod
@@ -17,6 +17,7 @@ require (
|
|||||||
github.com/stretchr/testify v1.8.4
|
github.com/stretchr/testify v1.8.4
|
||||||
go.uber.org/fx v1.20.1
|
go.uber.org/fx v1.20.1
|
||||||
golang.org/x/crypto v0.38.0
|
golang.org/x/crypto v0.38.0
|
||||||
|
gopkg.in/yaml.v3 v3.0.1
|
||||||
gorm.io/driver/sqlite v1.5.4
|
gorm.io/driver/sqlite v1.5.4
|
||||||
gorm.io/gorm v1.25.5
|
gorm.io/gorm v1.25.5
|
||||||
modernc.org/sqlite v1.28.0
|
modernc.org/sqlite v1.28.0
|
||||||
@@ -52,7 +53,6 @@ require (
|
|||||||
golang.org/x/text v0.25.0 // indirect
|
golang.org/x/text v0.25.0 // indirect
|
||||||
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
|
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
|
||||||
google.golang.org/protobuf v1.31.0 // indirect
|
google.golang.org/protobuf v1.31.0 // indirect
|
||||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
|
||||||
lukechampine.com/uint128 v1.2.0 // indirect
|
lukechampine.com/uint128 v1.2.0 // indirect
|
||||||
modernc.org/cc/v3 v3.40.0 // indirect
|
modernc.org/cc/v3 v3.40.0 // indirect
|
||||||
modernc.org/ccgo/v3 v3.16.13 // indirect
|
modernc.org/ccgo/v3 v3.16.13 // indirect
|
||||||
|
|||||||
387
internal/ciscript/ci_mark_superseded_test.go
Normal file
387
internal/ciscript/ci_mark_superseded_test.go
Normal file
@@ -0,0 +1,387 @@
|
|||||||
|
package ciscript_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"maps"
|
||||||
|
"os"
|
||||||
|
"os/exec"
|
||||||
|
"path/filepath"
|
||||||
|
"slices"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"gopkg.in/yaml.v3"
|
||||||
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
// supersededDesc is the description script/ci-mark-superseded
|
||||||
|
// writes, and the one an earlier revision of it wrote alongside a
|
||||||
|
// `skipped` state.
|
||||||
|
supersededDesc = "Superseded by a newer commit; never tested"
|
||||||
|
|
||||||
|
// liveContext is the commit-status context Gitea uses for this
|
||||||
|
// repository's runs, as seen in its API. The script derives it from
|
||||||
|
// the workflow and job names rather than hardcoding it; the
|
||||||
|
// derivation is checked against this value below.
|
||||||
|
liveContext = "check / check (push)"
|
||||||
|
|
||||||
|
scriptPath = "../../script/ci-mark-superseded"
|
||||||
|
workflow = "../../.gitea/workflows/check.yml"
|
||||||
|
|
||||||
|
// failure is the only state that neither folds into a combined
|
||||||
|
// `success` (as `skipped` does) nor blocks the commit forever (as
|
||||||
|
// `pending` does).
|
||||||
|
failure = "failure"
|
||||||
|
)
|
||||||
|
|
||||||
|
// repo is a throwaway git history: parent is the commit a run would be
|
||||||
|
// cancelled on, head the commit that superseded it.
|
||||||
|
type repo struct {
|
||||||
|
dir string
|
||||||
|
head string
|
||||||
|
parent string
|
||||||
|
}
|
||||||
|
|
||||||
|
// scriptEnv is the run identity the Gitea runner exports and the script
|
||||||
|
// builds its context string from.
|
||||||
|
type scriptEnv struct {
|
||||||
|
workflow string
|
||||||
|
job string
|
||||||
|
event string
|
||||||
|
}
|
||||||
|
|
||||||
|
func defaultEnv() scriptEnv {
|
||||||
|
return scriptEnv{workflow: "check", job: "check", event: "push"}
|
||||||
|
}
|
||||||
|
|
||||||
|
func cancelled() commitStatus {
|
||||||
|
return commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: failure,
|
||||||
|
Description: "Has been cancelled",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func running() commitStatus {
|
||||||
|
return commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: "pending",
|
||||||
|
Description: "Has started running",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMarkSuperseded(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cases := map[string]struct {
|
||||||
|
parent commitStatus
|
||||||
|
wantMark bool
|
||||||
|
}{
|
||||||
|
"a cancelled run is marked": {
|
||||||
|
parent: cancelled(),
|
||||||
|
wantMark: true,
|
||||||
|
},
|
||||||
|
"a laundered skipped status is marked": {
|
||||||
|
parent: commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: "skipped",
|
||||||
|
Description: supersededDesc,
|
||||||
|
},
|
||||||
|
wantMark: true,
|
||||||
|
},
|
||||||
|
"a genuine failure is left alone": {
|
||||||
|
parent: commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: failure,
|
||||||
|
Description: "Failing after 3m1s",
|
||||||
|
},
|
||||||
|
wantMark: false,
|
||||||
|
},
|
||||||
|
"a passing run is left alone": {
|
||||||
|
parent: commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: "success",
|
||||||
|
Description: "Successful in 2m52s",
|
||||||
|
},
|
||||||
|
wantMark: false,
|
||||||
|
},
|
||||||
|
"another context is left alone": {
|
||||||
|
parent: commitStatus{
|
||||||
|
Context: "other / other (push)",
|
||||||
|
Status: failure,
|
||||||
|
Description: "Has been cancelled",
|
||||||
|
},
|
||||||
|
wantMark: false,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for name, tc := range cases {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, tc.parent)
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, defaultEnv())
|
||||||
|
require.NoError(t, err, out)
|
||||||
|
|
||||||
|
posted := fake.postedFor(history.parent)
|
||||||
|
if !tc.wantMark {
|
||||||
|
require.Empty(t, posted)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Equal(t, []postedStatus{{
|
||||||
|
Context: liveContext,
|
||||||
|
// Not `skipped`: Gitea's combined status folds
|
||||||
|
// that into `success`, which is what made a
|
||||||
|
// never-tested commit read green.
|
||||||
|
State: failure,
|
||||||
|
Description: supersededDesc,
|
||||||
|
}}, posted)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A second run must not rewrite what the first one wrote, or every
|
||||||
|
// later push would post a duplicate status.
|
||||||
|
func TestMarkSupersededIsIdempotent(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
for range 2 {
|
||||||
|
out, err := runScript(t, history, api, defaultEnv())
|
||||||
|
require.NoError(t, err, out)
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Len(t, fake.postedFor(history.parent), 1)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Renaming the workflow or the job changes the context string Gitea
|
||||||
|
// uses. The script must say so instead of quietly matching nothing.
|
||||||
|
func TestMarkSupersededRejectsAnUnknownContext(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
env := defaultEnv()
|
||||||
|
env.job = "renamed"
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, env)
|
||||||
|
require.Error(t, err)
|
||||||
|
require.Contains(t, out, "renamed")
|
||||||
|
require.Contains(t, out, liveContext)
|
||||||
|
require.Empty(t, fake.postedFor(history.parent))
|
||||||
|
}
|
||||||
|
|
||||||
|
// ANCESTOR_LIMIT is a documented knob. A value that is set but unusable
|
||||||
|
// must abort: handing it to git and discarding the exit status left the
|
||||||
|
// walk empty and the step green, marking nothing.
|
||||||
|
func TestMarkSupersededRejectsAnUnparseableAncestorLimit(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
out, err := runScript(
|
||||||
|
t, history, api, defaultEnv(), "ANCESTOR_LIMIT=twenty",
|
||||||
|
)
|
||||||
|
require.Error(t, err)
|
||||||
|
require.Contains(t, out, "ANCESTOR_LIMIT")
|
||||||
|
require.Contains(t, out, "twenty")
|
||||||
|
require.Empty(t, fake.postedFor(history.parent))
|
||||||
|
}
|
||||||
|
|
||||||
|
// A status read that fails is not the same as a commit with nothing to
|
||||||
|
// do. Losing curl's exit status through a pipe made the two identical
|
||||||
|
// and left a laundered commit laundered with no signal.
|
||||||
|
func TestMarkSupersededFailsOnAnUnreadableAncestorStatus(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
fake.failStatusRead(history.parent)
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, defaultEnv())
|
||||||
|
require.Error(t, err)
|
||||||
|
require.Contains(t, out, history.parent)
|
||||||
|
require.Contains(t, out, "cannot read commit statuses")
|
||||||
|
require.Empty(t, fake.postedFor(history.parent))
|
||||||
|
}
|
||||||
|
|
||||||
|
// A shallow clone cannot resolve the parent, so it is indistinguishable
|
||||||
|
// from a root commit to rev-parse and the walk would exit 0 having
|
||||||
|
// marked nothing. It must abort instead: dropping `fetch-depth: 0` from
|
||||||
|
// the checkout step is one edit, and a silent no-op there restores the
|
||||||
|
// false-green bug this script exists to prevent.
|
||||||
|
func TestMarkSupersededRejectsAShallowRepository(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := shallowClone(t, newRepo(t))
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, defaultEnv())
|
||||||
|
require.Error(t, err)
|
||||||
|
require.Contains(t, out, "shallow repository")
|
||||||
|
require.Empty(t, fake.postedFor(history.parent))
|
||||||
|
require.Empty(t, fake.postedFor(history.head))
|
||||||
|
}
|
||||||
|
|
||||||
|
// shallowClone returns the same history as a depth-1 clone. The `file://`
|
||||||
|
// URL is required: git ignores --depth for a plain local path.
|
||||||
|
func shallowClone(t *testing.T, history repo) repo {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
//nolint:gosec // fixed argv, arguments are test-local paths
|
||||||
|
cmd := exec.CommandContext(t.Context(), "git", "clone", "-q",
|
||||||
|
"--depth=1", "file://"+history.dir, dir)
|
||||||
|
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
require.NoError(t, err, string(out))
|
||||||
|
|
||||||
|
return repo{dir: dir, head: history.head, parent: history.parent}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The derived context must equal the one Gitea actually uses, which is
|
||||||
|
// built from the same workflow and job names.
|
||||||
|
func TestDerivedContextMatchesGitea(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
name, job := workflowIdentity(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, scriptEnv{
|
||||||
|
workflow: name,
|
||||||
|
job: job,
|
||||||
|
event: "push",
|
||||||
|
})
|
||||||
|
require.NoError(t, err, out)
|
||||||
|
|
||||||
|
posted := fake.postedFor(history.parent)
|
||||||
|
require.Len(t, posted, 1)
|
||||||
|
require.Equal(t, liveContext, posted[0].Context)
|
||||||
|
}
|
||||||
|
|
||||||
|
// workflowIdentity reads the workflow name and its single job id out of
|
||||||
|
// the checked-in workflow file.
|
||||||
|
func workflowIdentity(t *testing.T) (string, string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
raw, err := os.ReadFile(workflow)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
var parsed struct {
|
||||||
|
Name string `yaml:"name"`
|
||||||
|
Jobs map[string]any `yaml:"jobs"`
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(t, yaml.Unmarshal(raw, &parsed))
|
||||||
|
|
||||||
|
jobs := slices.Collect(maps.Keys(parsed.Jobs))
|
||||||
|
require.Len(t, jobs, 1)
|
||||||
|
|
||||||
|
return parsed.Name, jobs[0]
|
||||||
|
}
|
||||||
|
|
||||||
|
func runScript(
|
||||||
|
t *testing.T, history repo, api string, env scriptEnv,
|
||||||
|
extra ...string,
|
||||||
|
) (string, error) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
script, err := filepath.Abs(scriptPath)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
//nolint:gosec // fixed argv, repo-local script under test
|
||||||
|
cmd := exec.CommandContext(t.Context(), "sh", script)
|
||||||
|
cmd.Dir = history.dir
|
||||||
|
cmd.Env = append(os.Environ(),
|
||||||
|
"GITHUB_API_URL="+api,
|
||||||
|
"GITHUB_REPOSITORY=sneak/webhooker",
|
||||||
|
"GITHUB_SHA="+history.head,
|
||||||
|
"GITHUB_WORKFLOW="+env.workflow,
|
||||||
|
"GITHUB_JOB="+env.job,
|
||||||
|
"GITHUB_EVENT_NAME="+env.event,
|
||||||
|
"GITEA_TOKEN=test-token",
|
||||||
|
)
|
||||||
|
cmd.Env = append(cmd.Env, extra...)
|
||||||
|
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
|
||||||
|
return string(out), err
|
||||||
|
}
|
||||||
|
|
||||||
|
func newRepo(t *testing.T) repo {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
git := func(args ...string) string {
|
||||||
|
//nolint:gosec // fixed argv, arguments are test constants
|
||||||
|
cmd := exec.CommandContext(t.Context(), "git", args...)
|
||||||
|
cmd.Dir = dir
|
||||||
|
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
require.NoError(t, err, string(out))
|
||||||
|
|
||||||
|
return strings.TrimSpace(string(out))
|
||||||
|
}
|
||||||
|
|
||||||
|
commit := func(message string) string {
|
||||||
|
git(
|
||||||
|
"-c", "user.email=ci@example.invalid",
|
||||||
|
"-c", "user.name=ci",
|
||||||
|
"-c", "commit.gpgsign=false",
|
||||||
|
"commit", "-q", "--allow-empty", "-m", message,
|
||||||
|
)
|
||||||
|
|
||||||
|
return git("rev-parse", "HEAD")
|
||||||
|
}
|
||||||
|
|
||||||
|
git("init", "-q", "-b", "main")
|
||||||
|
|
||||||
|
parent := commit("parent")
|
||||||
|
head := commit("head")
|
||||||
|
|
||||||
|
return repo{dir: dir, head: head, parent: parent}
|
||||||
|
}
|
||||||
|
|
||||||
|
func requireTools(t *testing.T) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
for _, tool := range []string{"sh", "git", "curl", "jq"} {
|
||||||
|
_, err := exec.LookPath(tool)
|
||||||
|
if err != nil {
|
||||||
|
t.Skipf("%s is not installed: %v", tool, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
10
internal/ciscript/doc.go
Normal file
10
internal/ciscript/doc.go
Normal file
@@ -0,0 +1,10 @@
|
|||||||
|
// Package ciscript holds the tests for the repository's CI shell
|
||||||
|
// scripts in script/. It carries no runtime code: the scripts run on
|
||||||
|
// the CI runner, not inside the binary, but their behaviour still has
|
||||||
|
// to be verified by the test suite.
|
||||||
|
//
|
||||||
|
// The scripts under test are outside the Go build graph, so `go test`'s
|
||||||
|
// result cache serves a stale PASS when only a script changed: run the
|
||||||
|
// container build, or GOFLAGS=-count=1, to trust a result here after
|
||||||
|
// editing script/.
|
||||||
|
package ciscript
|
||||||
162
internal/ciscript/fakegitea_test.go
Normal file
162
internal/ciscript/fakegitea_test.go
Normal file
@@ -0,0 +1,162 @@
|
|||||||
|
package ciscript_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"sync"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// commitStatus is the part of an entry in Gitea's combined-status
|
||||||
|
// response that script/ci-mark-superseded reads.
|
||||||
|
type commitStatus struct {
|
||||||
|
Context string `json:"context"`
|
||||||
|
Status string `json:"status"`
|
||||||
|
Description string `json:"description"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// postedStatus is the part of a create-status request body the script
|
||||||
|
// writes.
|
||||||
|
type postedStatus struct {
|
||||||
|
Context string `json:"context"`
|
||||||
|
State string `json:"state"`
|
||||||
|
Description string `json:"description"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// fakeGitea serves the two endpoints the script talks to. Like Gitea,
|
||||||
|
// the newest status for a context replaces the previous one, so a
|
||||||
|
// second run of the script sees what the first one wrote.
|
||||||
|
type fakeGitea struct {
|
||||||
|
mu sync.Mutex
|
||||||
|
statuses map[string][]commitStatus
|
||||||
|
posted map[string][]postedStatus
|
||||||
|
// failRead is a commit whose combined-status read answers HTTP
|
||||||
|
// 500, standing in for a status API that is down.
|
||||||
|
failRead string
|
||||||
|
}
|
||||||
|
|
||||||
|
// newFakeGitea returns the fake and the base URL to hand the script as
|
||||||
|
// GITHUB_API_URL.
|
||||||
|
func newFakeGitea(t *testing.T) (*fakeGitea, string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
fake := &fakeGitea{
|
||||||
|
mu: sync.Mutex{},
|
||||||
|
statuses: map[string][]commitStatus{},
|
||||||
|
posted: map[string][]postedStatus{},
|
||||||
|
failRead: "",
|
||||||
|
}
|
||||||
|
|
||||||
|
srv := httptest.NewServer(fake.routes())
|
||||||
|
t.Cleanup(srv.Close)
|
||||||
|
|
||||||
|
return fake, srv.URL
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *fakeGitea) routes() http.Handler {
|
||||||
|
mux := http.NewServeMux()
|
||||||
|
|
||||||
|
mux.HandleFunc(
|
||||||
|
"GET /repos/{owner}/{repo}/commits/{sha}/status",
|
||||||
|
f.handleCombined,
|
||||||
|
)
|
||||||
|
mux.HandleFunc(
|
||||||
|
"POST /repos/{owner}/{repo}/statuses/{sha}",
|
||||||
|
f.handleCreate,
|
||||||
|
)
|
||||||
|
|
||||||
|
return mux
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *fakeGitea) handleCombined(
|
||||||
|
w http.ResponseWriter, r *http.Request,
|
||||||
|
) {
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
sha := r.PathValue("sha")
|
||||||
|
if f.failRead != "" && f.failRead == sha {
|
||||||
|
http.Error(w, "boom", http.StatusInternalServerError)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
body := struct {
|
||||||
|
Statuses []commitStatus `json:"statuses"`
|
||||||
|
}{Statuses: f.statuses[sha]}
|
||||||
|
|
||||||
|
payload, err := json.Marshal(body)
|
||||||
|
if err != nil {
|
||||||
|
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
w.Header().Set("Content-Type", "application/json")
|
||||||
|
|
||||||
|
_, _ = w.Write(payload)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *fakeGitea) handleCreate(w http.ResponseWriter, r *http.Request) {
|
||||||
|
var got postedStatus
|
||||||
|
|
||||||
|
err := json.NewDecoder(r.Body).Decode(&got)
|
||||||
|
if err != nil {
|
||||||
|
http.Error(w, err.Error(), http.StatusBadRequest)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
sha := r.PathValue("sha")
|
||||||
|
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
f.posted[sha] = append(f.posted[sha], got)
|
||||||
|
f.replaceLocked(sha, commitStatus{
|
||||||
|
Context: got.Context,
|
||||||
|
Status: got.State,
|
||||||
|
Description: got.Description,
|
||||||
|
})
|
||||||
|
|
||||||
|
w.WriteHeader(http.StatusCreated)
|
||||||
|
}
|
||||||
|
|
||||||
|
// failStatusRead makes the combined-status read for one commit answer
|
||||||
|
// HTTP 500.
|
||||||
|
func (f *fakeGitea) failStatusRead(sha string) {
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
f.failRead = sha
|
||||||
|
}
|
||||||
|
|
||||||
|
// setStatus gives a commit its latest status for a context.
|
||||||
|
func (f *fakeGitea) setStatus(sha string, status commitStatus) {
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
f.replaceLocked(sha, status)
|
||||||
|
}
|
||||||
|
|
||||||
|
// postedFor returns the statuses the script created for a commit.
|
||||||
|
func (f *fakeGitea) postedFor(sha string) []postedStatus {
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
return append([]postedStatus(nil), f.posted[sha]...)
|
||||||
|
}
|
||||||
|
|
||||||
|
// replaceLocked requires f.mu.
|
||||||
|
func (f *fakeGitea) replaceLocked(sha string, status commitStatus) {
|
||||||
|
for i, existing := range f.statuses[sha] {
|
||||||
|
if existing.Context == status.Context {
|
||||||
|
f.statuses[sha][i] = status
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
f.statuses[sha] = append(f.statuses[sha], status)
|
||||||
|
}
|
||||||
152
script/ci-mark-superseded
Executable file
152
script/ci-mark-superseded
Executable file
@@ -0,0 +1,152 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# script/ci-mark-superseded: record an honest status on commits whose CI
|
||||||
|
# run Gitea cancelled because a newer commit landed on the same branch.
|
||||||
|
# Gitea writes `failure` / "Has been cancelled" for such a run, which
|
||||||
|
# reads as a test result on a commit nothing ever tested. Cancellation is
|
||||||
|
# unconditional server-side for push events, so the superseding run
|
||||||
|
# rewrites those statuses to `failure` with a description that says the
|
||||||
|
# commit was never tested. `skipped` cannot be used: Gitea's combined
|
||||||
|
# status folds `skipped` into `success`, so a never-tested commit would
|
||||||
|
# report green. Genuine failures and successes are never touched.
|
||||||
|
#
|
||||||
|
# Called by the Gitea Actions workflow, which supplies GITHUB_API_URL,
|
||||||
|
# GITHUB_REPOSITORY, GITHUB_SHA, GITHUB_WORKFLOW, GITHUB_JOB,
|
||||||
|
# GITHUB_EVENT_NAME and GITEA_TOKEN. ANCESTOR_LIMIT (default 20) caps how
|
||||||
|
# far back the walk looks; a value that is set but not a positive integer
|
||||||
|
# aborts rather than silently disabling the walk.
|
||||||
|
set -eu
|
||||||
|
|
||||||
|
SUPERSEDED_DESC='Superseded by a newer commit; never tested'
|
||||||
|
|
||||||
|
# Gitea builds the commit-status context as
|
||||||
|
# "<workflow name> / <job name> (<event>)", so derive it rather than
|
||||||
|
# hardcoding the result.
|
||||||
|
#
|
||||||
|
# The derivation is deliberately not byte-exact with Gitea's own rule and
|
||||||
|
# must not be "fixed" into a silent fallback. Gitea uses the job's `name:`
|
||||||
|
# (falling back to the job id) and the workflow's `name:` (falling back to
|
||||||
|
# the workflow filename), while the runner exports GITHUB_JOB as the job
|
||||||
|
# *id* and GITHUB_WORKFLOW as the parsed workflow `name:`. So giving the
|
||||||
|
# job a display `name:`, or dropping the workflow's `name:`, makes the
|
||||||
|
# derived context stop matching --- and require_own_context below then
|
||||||
|
# turns every push red with a message. That loud failure is the point
|
||||||
|
# (https://git.eeqj.de/sneak/webhooker/issues/147 item 2); guessing at a
|
||||||
|
# fallback would restore the silent no-op it replaced.
|
||||||
|
context() {
|
||||||
|
printf '%s / %s (%s)' \
|
||||||
|
"$GITHUB_WORKFLOW" "$GITHUB_JOB" "$GITHUB_EVENT_NAME"
|
||||||
|
}
|
||||||
|
|
||||||
|
# ANCESTOR_LIMIT is a documented knob, so a value that is set but
|
||||||
|
# unusable must fail loudly instead of defaulting
|
||||||
|
# (https://git.eeqj.de/sneak/webhooker/issues/80). Passing it straight to
|
||||||
|
# git would print `fatal: not an integer` into a discarded exit status
|
||||||
|
# and mark nothing.
|
||||||
|
ancestor_limit() {
|
||||||
|
# `-` and not `:-`: an explicitly empty value is set-but-unusable
|
||||||
|
# config, so it aborts like any other bad value rather than silently
|
||||||
|
# running at the default.
|
||||||
|
_limit="${ANCESTOR_LIMIT-20}"
|
||||||
|
case "$_limit" in
|
||||||
|
'' | *[!0-9]* | 0*)
|
||||||
|
echo "ANCESTOR_LIMIT must be a positive integer," \
|
||||||
|
"got '${_limit}'" >&2
|
||||||
|
|
||||||
|
return 1
|
||||||
|
;;
|
||||||
|
esac
|
||||||
|
printf '%s' "$_limit"
|
||||||
|
}
|
||||||
|
|
||||||
|
# The status Gitea created for this very job proves which context string
|
||||||
|
# it uses. If the derived one is missing, the workflow or the job was
|
||||||
|
# renamed and the match below would silently stop firing, restoring the
|
||||||
|
# false-red bug with no signal. Fail loudly instead.
|
||||||
|
require_own_context() {
|
||||||
|
if ! _body="$(curl -sf --retry 3 --retry-delay 2 --max-time 30 \
|
||||||
|
"${1}/commits/${GITHUB_SHA}/status")"; then
|
||||||
|
echo "cannot read commit statuses for ${GITHUB_SHA}" >&2
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
_found="$(printf '%s' "$_body" | jq -r '(.statuses // [])[].context')"
|
||||||
|
if printf '%s\n' "$_found" | grep -qxF "$2"; then
|
||||||
|
return 0
|
||||||
|
fi
|
||||||
|
echo "no commit status with context '${2}' on ${GITHUB_SHA}:" >&2
|
||||||
|
echo "workflow or job renamed? contexts present:" >&2
|
||||||
|
printf '%s\n' "$_found" >&2
|
||||||
|
return 1
|
||||||
|
}
|
||||||
|
|
||||||
|
# Latest status for our context on a commit, as "state|description".
|
||||||
|
# The read is retried and bounded, and a read that still fails aborts the
|
||||||
|
# step: a laundered commit that cannot be read is not the same as one
|
||||||
|
# with nothing to do, and piping curl into jq would discard the
|
||||||
|
# difference.
|
||||||
|
status_of() {
|
||||||
|
if ! _sbody="$(curl -sf --retry 3 --retry-delay 2 --max-time 30 \
|
||||||
|
"${1}/commits/${2}/status")"; then
|
||||||
|
echo "cannot read commit statuses for ${2}" >&2
|
||||||
|
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
printf '%s' "$_sbody" | jq -r --arg c "$3" \
|
||||||
|
'[(.statuses // [])[] | select(.context == $c)][0] // empty
|
||||||
|
| "\(.status)|\(.description)"'
|
||||||
|
}
|
||||||
|
|
||||||
|
mark_superseded() {
|
||||||
|
curl -sf -X POST "${1}/statuses/${2}" \
|
||||||
|
-H "Authorization: token ${GITEA_TOKEN}" \
|
||||||
|
-H 'Content-Type: application/json' \
|
||||||
|
-d "$(jq -nc --arg c "$3" --arg d "$SUPERSEDED_DESC" \
|
||||||
|
'{context: $c, state: "failure", description: $d}')" \
|
||||||
|
>/dev/null
|
||||||
|
}
|
||||||
|
|
||||||
|
main() {
|
||||||
|
_api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
|
||||||
|
_ctx="$(context)"
|
||||||
|
|
||||||
|
_limit="$(ancestor_limit)"
|
||||||
|
|
||||||
|
require_own_context "$_api" "$_ctx"
|
||||||
|
|
||||||
|
# A shallow clone cannot resolve the parent, so it looks exactly like
|
||||||
|
# a root commit to rev-parse below and would exit 0 having walked
|
||||||
|
# nothing (or, at depth > 1, only the ancestors that happen to be
|
||||||
|
# present). The workflow checks out with `fetch-depth: 0`; verify
|
||||||
|
# that here rather than depend on it silently.
|
||||||
|
if [ "$(git rev-parse --is-shallow-repository)" = 'true' ]; then
|
||||||
|
echo "shallow repository: the ancestor walk needs full history" >&2
|
||||||
|
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# A root commit legitimately has no ancestors and is not an error.
|
||||||
|
# A SHA this repository does not have lands here too, since its
|
||||||
|
# parent is equally unresolvable, but require_own_context above has
|
||||||
|
# already aborted on the 404 for it. The walk itself carries no
|
||||||
|
# `|| true`, so a rev-list failure aborts.
|
||||||
|
if ! git rev-parse -q --verify "${GITHUB_SHA}^" >/dev/null; then
|
||||||
|
echo "no ancestor of ${GITHUB_SHA} to check"
|
||||||
|
|
||||||
|
return 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
_walk="$(git rev-list --max-count="$_limit" "${GITHUB_SHA}^")"
|
||||||
|
|
||||||
|
for _sha in $_walk; do
|
||||||
|
_latest="$(status_of "$_api" "$_sha" "$_ctx")"
|
||||||
|
# A run that was cancelled, or one an earlier revision of this
|
||||||
|
# script laundered into `skipped`. Anything else stands.
|
||||||
|
case "$_latest" in
|
||||||
|
'failure|Has been cancelled' | "skipped|${SUPERSEDED_DESC}") ;;
|
||||||
|
*) continue ;;
|
||||||
|
esac
|
||||||
|
mark_superseded "$_api" "$_sha" "$_ctx"
|
||||||
|
echo "marked superseded: ${_sha}"
|
||||||
|
done
|
||||||
|
}
|
||||||
|
|
||||||
|
main "$@"
|
||||||
Reference in New Issue
Block a user