Mark superseded commits honestly instead of skipped (closes #152)
All checks were successful
check / check (push) Successful in 2m52s
All checks were successful
check / check (push) Successful in 2m52s
Gitea cancels an in-flight run when a newer commit lands on the same branch and records the cancellation as `failure` / "Has been cancelled". The workflow rewrote that to `skipped`, but Gitea's Combine() folds `skipped` into `success`, so the combined-status API returned green for a commit nothing had ever tested. Rewrite it to `failure` / "Superseded by a newer commit; never tested" instead: red-but-honest, and never `pending`, which would block the commit forever. Re-running the superseded commit would have been better still, but is not reachable on this Gitea (1.25.4): its API exposes no rerun endpoint, workflow dispatch takes a ref rather than a SHA, and every replay would be a full uncached build with no bound on how many pile up behind a burst of merges. The step also stops hardcoding its status context: the logic moves into script/ci-mark-superseded, which derives the context from the workflow name, job id and event, and fails loudly when no status on the commit being built carries that context, so renaming the workflow or the job cannot silently disable the rewrite. The derivation is not byte-exact with Gitea's own rule -- Gitea uses the job's display `name:` where the runner exports the job id -- so adding a `name:` to the job turns every push red rather than quietly doing nothing; the script header says so, because that loud failure is the point. That is item 2 of #147; item 1 there is untouched. Nothing about the walk may fail quietly, since the script exists to stop CI lying quietly. An ANCESTOR_LIMIT that is set but not a positive integer aborts instead of passing an unusable value to git and discarding the error; the root-commit case is detected explicitly so every other rev-list failure aborts too; and per-ancestor status reads carry the same `--retry 3 --max-time 30` as the head-commit read and abort on failure rather than losing curl's exit status through a pipe. Tests drive the script against a fake Gitea covering the cancelled, laundered-skipped, genuinely-failed, passing and renamed cases, an unparseable ANCESTOR_LIMIT and an ancestor whose status read answers HTTP 500, so jq joins the builder image to run them.
This commit is contained in:
349
internal/ciscript/ci_mark_superseded_test.go
Normal file
349
internal/ciscript/ci_mark_superseded_test.go
Normal file
@@ -0,0 +1,349 @@
|
||||
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))
|
||||
}
|
||||
|
||||
// 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)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user