Commit 2f49d2d95e
Verified · cmc ci/build: success ci/sonar: success ci/test: success ci/vuln: success
Layout: unified · split
e2e/ci_skipped_test.go added +70
| @@ -0,0 +1,70 @@ | ||
| 1 | package e2e | |
| 2 | ||
| 3 | import ( | |
| 4 | "os" | |
| 5 | "path/filepath" | |
| 6 | "strings" | |
| 7 | "testing" | |
| 8 | ) | |
| 9 | ||
| 10 | // A repository with require_checks on and a job whose paths-ignore excludes | |
| 11 | // a docs-only change used to be unmergeable: the push queued nothing, so the | |
| 12 | // head carried no statuses at all, and the gate refuses that outright. The | |
| 13 | // filtered job now records a "skipped" status instead, which the gate reads | |
| 14 | // as green (#172). | |
| 15 | func TestSkippedStatusSatisfiesRequireChecks(t *testing.T) { | |
| 16 | inst := startInstance(t) | |
| 17 | aliceKey := inst.newKey(t, "alice") | |
| 18 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub", | |
| 19 | "--email", "alice@example.test", "--verified") | |
| 20 | ||
| 21 | if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 { | |
| 22 | t.Fatalf("repo create: %s", errOut) | |
| 23 | } | |
| 24 | if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-checks", "alice/app", "on"); code != 0 { | |
| 25 | t.Fatalf("require-checks on: %s", errOut) | |
| 26 | } | |
| 27 | ||
| 28 | work := t.TempDir() | |
| 29 | env := inst.gitEnv(aliceKey) | |
| 30 | mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w") | |
| 31 | dir := filepath.Join(work, "w") | |
| 32 | os.MkdirAll(filepath.Join(dir, ".gitbay"), 0o755) | |
| 33 | os.MkdirAll(filepath.Join(dir, "docs"), 0o755) | |
| 34 | os.WriteFile(filepath.Join(dir, ".gitbay", "ci.yml"), []byte( | |
| 35 | "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo hi\n"), 0o644) | |
| 36 | os.WriteFile(filepath.Join(dir, "docs", "x.md"), []byte("# x\n"), 0o644) | |
| 37 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | |
| 38 | mustGit(t, dir, env, "add", ".") | |
| 39 | mustGit(t, dir, env, "commit", "-q", "-m", "base") | |
| 40 | mustGit(t, dir, env, "push", "-q", "origin", "main") | |
| 41 | ||
| 42 | // A docs-only feature branch: the job's paths-ignore excludes every | |
| 43 | // file this push touches. | |
| 44 | mustGit(t, dir, env, "checkout", "-q", "-b", "docs-fix") | |
| 45 | os.WriteFile(filepath.Join(dir, "docs", "x.md"), []byte("# x, fixed\n"), 0o644) | |
| 46 | mustGit(t, dir, env, "add", ".") | |
| 47 | mustGit(t, dir, env, "commit", "-q", "-m", "fix a typo") | |
| 48 | mustGit(t, dir, env, "push", "-q", "origin", "docs-fix") | |
| 49 | headSHA := strings.TrimSpace(mustGit(t, dir, env, "rev-parse", "HEAD")) | |
| 50 | ||
| 51 | status, _, code := inst.ssh(t, aliceKey, "", "status", "list", "alice/app", headSHA) | |
| 52 | if code != 0 { | |
| 53 | t.Fatalf("status list: %s", status) | |
| 54 | } | |
| 55 | if !strings.Contains(status, "ci/unit") || !strings.Contains(status, "skipped") { | |
| 56 | t.Fatalf("no skipped ci/unit status on %s:\n%s", headSHA, status) | |
| 57 | } | |
| 58 | ||
| 59 | if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/app", | |
| 60 | "--source", "docs-fix", "--target", "main", "--title", "'fix a typo'"); code != 0 { | |
| 61 | t.Fatalf("mr create: %s", errOut) | |
| 62 | } | |
| 63 | ||
| 64 | // Before #172 this failed with "requires green checks and none were | |
| 65 | // reported": the filtered push left the head with no statuses at all. | |
| 66 | if out, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "1", | |
| 67 | "--strategy", "merge"); code != 0 { | |
| 68 | t.Fatalf("mr merge refused a docs-only change under require_checks: %s %s", out, errOut) | |
| 69 | } | |
| 70 | } | |
internal/control/build.go +24
| @@ -532,6 +532,26 @@ func QueueMRBuilds( | ||
| 532 | 532 | queueJobs(st, root, siteURL, repo, userID, mrHeadRef(n), "", sha, time.Now(), false, false, false) |
| 533 | 533 | } |
| 534 | 534 | |
| 535 | // skipReason names why a job's path filters excluded this push, mirroring | |
| 536 | // the order ci.Selected checks them in: an unmatched paths list rules a | |
| 537 | // job out before paths-ignore is even considered. | |
| 538 | func skipReason(j ci.Job, changed []string) string { | |
| 539 | if len(j.Paths) > 0 { | |
| 540 | hit := false | |
| 541 | for _, f := range changed { | |
| 542 | for _, p := range j.Paths { | |
| 543 | if ci.Match(p, f) { | |
| 544 | hit = true | |
| 545 | } | |
| 546 | } | |
| 547 | } | |
| 548 | if !hit { | |
| 549 | return "no changed file matches paths" | |
| 550 | } | |
| 551 | } | |
| 552 | return "every changed file matched paths-ignore" | |
| 553 | } | |
| 554 | ||
| 535 | 555 | func queueJobs( |
| 536 | 556 | st *store.Store, root, siteURL string, |
| 537 | 557 | repo store.Repo, userID int64, ref, old, sha string, now time.Time, |
| @@ -609,7 +629,11 @@ func queueJobs( | ||
| 609 | 629 | } |
| 610 | 630 | continue |
| 611 | 631 | } |
| 632 | // A filter that excludes this push is not silence: it satisfies | |
| 633 | // require_checks with a skipped status instead of leaving the | |
| 634 | // commit with none at all, which the gate refuses outright (#172). | |
| 612 | 635 | if filtered && !ci.Selected(j, changed) { |
| 636 | st.SetCommitStatus(repo.ID, sha, "ci/"+j.Name, "skipped", skipReason(j, changed), "", userID) | |
| 613 | 637 | continue |
| 614 | 638 | } |
| 615 | 639 | steps, _ := json.Marshal(j.Steps) |
internal/control/build_test.go +150
| @@ -301,6 +301,156 @@ func TestQueueBranchBuildsOrdinaryPushStillFilters(t *testing.T) { | ||
| 301 | 301 | } |
| 302 | 302 | } |
| 303 | 303 | |
| 304 | // A job a path filter excludes records a ci/<job> status of "skipped" | |
| 305 | // naming the reason, rather than leaving the commit with no status for | |
| 306 | // that job at all (#172). | |
| 307 | func TestQueueBranchBuildsFilteredJobRecordsSkippedStatus(t *testing.T) { | |
| 308 | st, repo, uid := newQueueTestRepo(t) | |
| 309 | git := gitRunner(t) | |
| 310 | root := t.TempDir() | |
| 311 | ||
| 312 | src := filepath.Join(root, "src") | |
| 313 | os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755) | |
| 314 | os.MkdirAll(filepath.Join(src, "docs"), 0o755) | |
| 315 | os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte( | |
| 316 | "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo hi\n"), 0o644) | |
| 317 | os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x\n"), 0o644) | |
| 318 | git(root, "init", "-q", "-b", "main", "src") | |
| 319 | git(src, "add", ".") | |
| 320 | git(src, "commit", "-q", "-m", "base") | |
| 321 | oldSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | |
| 322 | ||
| 323 | os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x changed\n"), 0o644) | |
| 324 | git(src, "add", ".") | |
| 325 | git(src, "commit", "-q", "-m", "docs only") | |
| 326 | newSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | |
| 327 | ||
| 328 | dir := RepoDir(root, repo.OwnerName, repo.Name) | |
| 329 | os.MkdirAll(filepath.Dir(dir), 0o755) | |
| 330 | git(root, "clone", "-q", "--bare", src, dir) | |
| 331 | ||
| 332 | QueueBranchBuilds(st, root, "https://x.test", repo, uid, "main", oldSHA, newSHA, time.Now()) | |
| 333 | ||
| 334 | statuses, err := st.ListCommitStatuses(repo.ID, newSHA) | |
| 335 | if err != nil { | |
| 336 | t.Fatal(err) | |
| 337 | } | |
| 338 | if len(statuses) != 1 || statuses[0].Context != "ci/unit" { | |
| 339 | t.Fatalf("statuses on %s: %+v", newSHA, statuses) | |
| 340 | } | |
| 341 | if statuses[0].State != "skipped" { | |
| 342 | t.Fatalf("filtered job status: got %q, want skipped", statuses[0].State) | |
| 343 | } | |
| 344 | if statuses[0].Description == "" { | |
| 345 | t.Fatal("skipped status carries no reason") | |
| 346 | } | |
| 347 | } | |
| 348 | ||
| 349 | // A tag job and a scheduled job are not push jobs at all: neither records | |
| 350 | // a skipped status, filtered push or not. Only the ordinary job that a | |
| 351 | // path filter actually excluded does. | |
| 352 | func TestQueueBranchBuildsTagAndScheduledJobsRecordNoSkippedStatus(t *testing.T) { | |
| 353 | st, repo, uid := newQueueTestRepo(t) | |
| 354 | git := gitRunner(t) | |
| 355 | root := t.TempDir() | |
| 356 | ||
| 357 | src := filepath.Join(root, "src") | |
| 358 | os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755) | |
| 359 | os.MkdirAll(filepath.Join(src, "docs"), 0o755) | |
| 360 | os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte(strings.Join([]string{ | |
| 361 | "jobs:", | |
| 362 | " unit:", | |
| 363 | " paths-ignore:", | |
| 364 | " - docs/**", | |
| 365 | " steps:", | |
| 366 | " - echo hi", | |
| 367 | " release:", | |
| 368 | " tags: 'v*'", | |
| 369 | " steps:", | |
| 370 | " - echo release", | |
| 371 | " nightly:", | |
| 372 | " schedule: '0 0 * * *'", | |
| 373 | " steps:", | |
| 374 | " - echo nightly", | |
| 375 | "", | |
| 376 | }, "\n")), 0o644) | |
| 377 | os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x\n"), 0o644) | |
| 378 | git(root, "init", "-q", "-b", "main", "src") | |
| 379 | git(src, "add", ".") | |
| 380 | git(src, "commit", "-q", "-m", "base") | |
| 381 | oldSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | |
| 382 | ||
| 383 | os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x changed\n"), 0o644) | |
| 384 | git(src, "add", ".") | |
| 385 | git(src, "commit", "-q", "-m", "docs only") | |
| 386 | newSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | |
| 387 | ||
| 388 | dir := RepoDir(root, repo.OwnerName, repo.Name) | |
| 389 | os.MkdirAll(filepath.Dir(dir), 0o755) | |
| 390 | git(root, "clone", "-q", "--bare", src, dir) | |
| 391 | ||
| 392 | QueueBranchBuilds(st, root, "https://x.test", repo, uid, "main", oldSHA, newSHA, time.Now()) | |
| 393 | ||
| 394 | statuses, err := st.ListCommitStatuses(repo.ID, newSHA) | |
| 395 | if err != nil { | |
| 396 | t.Fatal(err) | |
| 397 | } | |
| 398 | if len(statuses) != 1 || statuses[0].Context != "ci/unit" || statuses[0].State != "skipped" { | |
| 399 | t.Fatalf("statuses on %s: %+v", newSHA, statuses) | |
| 400 | } | |
| 401 | } | |
| 402 | ||
| 403 | // A job already built for this commit on another branch is a fact about | |
| 404 | // the commit, not something a filter excluded: it keeps whatever status | |
| 405 | // that build reported (or none, if the run is still queued elsewhere) and | |
| 406 | // must not be overwritten with "skipped". | |
| 407 | func TestQueueBranchBuildsAlreadyBuiltJobRecordsNoSkippedStatus(t *testing.T) { | |
| 408 | st, repo, uid := newQueueTestRepo(t) | |
| 409 | git := gitRunner(t) | |
| 410 | root := t.TempDir() | |
| 411 | ||
| 412 | src := filepath.Join(root, "src") | |
| 413 | os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755) | |
| 414 | os.MkdirAll(filepath.Join(src, "docs"), 0o755) | |
| 415 | os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte( | |
| 416 | "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo hi\n"), 0o644) | |
| 417 | os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x\n"), 0o644) | |
| 418 | git(root, "init", "-q", "-b", "main", "src") | |
| 419 | git(src, "add", ".") | |
| 420 | git(src, "commit", "-q", "-m", "base") | |
| 421 | oldSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | |
| 422 | ||
| 423 | os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x changed\n"), 0o644) | |
| 424 | git(src, "add", ".") | |
| 425 | git(src, "commit", "-q", "-m", "docs only") | |
| 426 | newSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | |
| 427 | ||
| 428 | dir := RepoDir(root, repo.OwnerName, repo.Name) | |
| 429 | os.MkdirAll(filepath.Dir(dir), 0o755) | |
| 430 | git(root, "clone", "-q", "--bare", src, dir) | |
| 431 | ||
| 432 | // The same commit already has a build for "unit" from another branch, | |
| 433 | // still pending. Its filter would exclude this push too, so the only | |
| 434 | // way to tell the two paths apart is that this one must record nothing. | |
| 435 | if _, err := st.CreateBuild(repo.ID, "unit", newSHA, "other", `["echo hi"]`, true); err != nil { | |
| 436 | t.Fatal(err) | |
| 437 | } | |
| 438 | ||
| 439 | QueueBranchBuilds(st, root, "https://x.test", repo, uid, "main", oldSHA, newSHA, time.Now()) | |
| 440 | ||
| 441 | statuses, err := st.ListCommitStatuses(repo.ID, newSHA) | |
| 442 | if err != nil { | |
| 443 | t.Fatal(err) | |
| 444 | } | |
| 445 | if len(statuses) != 0 { | |
| 446 | t.Fatalf("already-built job recorded a status: %+v", statuses) | |
| 447 | } | |
| 448 | builds, err := st.ListBuilds(repo.ID, 10) | |
| 449 | if err != nil || len(builds) != 1 { | |
| 450 | t.Fatalf("expected only the pre-existing build: %v %v", builds, err) | |
| 451 | } | |
| 452 | } | |
| 453 | ||
| 304 | 454 | // QueueMRBuilds keeps failing open with no diff base at all: deriving a |
| 305 | 455 | // merge base for the MR head is deliberately out of scope here (#172 — |
| 306 | 456 | // filtering a head down to zero jobs leaves it with no statuses, which |
internal/httpd/mrpage_test.go +22
| @@ -109,3 +109,25 @@ func TestMRAsideTimestamps(t *testing.T) { | ||
| 109 | 109 | } |
| 110 | 110 | } |
| 111 | 111 | } |
| 112 | ||
| 113 | // A job a path filter excluded has no build behind its status: the page | |
| 114 | // must show it as skipped rather than linking to a build that never ran | |
| 115 | // (#172). | |
| 116 | func TestMRChecksRenderSkippedWithoutBuildLink(t *testing.T) { | |
| 117 | out := renderMR(t, testMR("open"), nil, []store.Check{ | |
| 118 | {CommitStatus: store.CommitStatus{Context: "ci/unit", State: "skipped", | |
| 119 | Description: "every changed file matched paths-ignore", UpdatedAt: "2026-08-27T14:05:00.000Z"}}, | |
| 120 | }) | |
| 121 | row := "" | |
| 122 | for _, line := range strings.Split(out, "\n") { | |
| 123 | if strings.Contains(line, "ci/unit") { | |
| 124 | row = line | |
| 125 | } | |
| 126 | } | |
| 127 | if row == "" || !strings.Contains(row, ">skipped<") { | |
| 128 | t.Fatalf("skipped check not rendered:\n%s", out) | |
| 129 | } | |
| 130 | if strings.Contains(row, "<a href") { | |
| 131 | t.Errorf("skipped check with no build linked anyway: %s", row) | |
| 132 | } | |
| 133 | } | |
internal/store/migrations/0041_status_skipped.down.sql added +24
| @@ -0,0 +1,24 @@ | ||
| 1 | ALTER TABLE commit_statuses RENAME TO commit_statuses_old; | |
| 2 | ||
| 3 | CREATE TABLE commit_statuses ( | |
| 4 | id INTEGER PRIMARY KEY, | |
| 5 | repo_id INTEGER NOT NULL REFERENCES repos(id) ON DELETE CASCADE, | |
| 6 | commit_sha TEXT NOT NULL, | |
| 7 | context TEXT NOT NULL, | |
| 8 | state TEXT NOT NULL CHECK (state IN ('pending','success','failure','error')), | |
| 9 | description TEXT NOT NULL DEFAULT '', | |
| 10 | target_url TEXT NOT NULL DEFAULT '', | |
| 11 | creator_id INTEGER REFERENCES users(id) ON DELETE SET NULL, | |
| 12 | created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), | |
| 13 | updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), | |
| 14 | UNIQUE (repo_id, commit_sha, context) | |
| 15 | ); | |
| 16 | ||
| 17 | INSERT INTO commit_statuses (id, repo_id, commit_sha, context, state, description, target_url, creator_id, created_at, updated_at) | |
| 18 | SELECT id, repo_id, commit_sha, context, state, description, target_url, creator_id, created_at, updated_at | |
| 19 | FROM commit_statuses_old | |
| 20 | WHERE state != 'skipped'; | |
| 21 | ||
| 22 | DROP TABLE commit_statuses_old; | |
| 23 | ||
| 24 | CREATE INDEX commit_statuses_sha ON commit_statuses(repo_id, commit_sha); | |
internal/store/migrations/0041_status_skipped.up.sql added +23
| @@ -0,0 +1,23 @@ | ||
| 1 | ALTER TABLE commit_statuses RENAME TO commit_statuses_old; | |
| 2 | ||
| 3 | CREATE TABLE commit_statuses ( | |
| 4 | id INTEGER PRIMARY KEY, | |
| 5 | repo_id INTEGER NOT NULL REFERENCES repos(id) ON DELETE CASCADE, | |
| 6 | commit_sha TEXT NOT NULL, | |
| 7 | context TEXT NOT NULL, | |
| 8 | state TEXT NOT NULL CHECK (state IN ('pending','success','failure','error','skipped')), | |
| 9 | description TEXT NOT NULL DEFAULT '', | |
| 10 | target_url TEXT NOT NULL DEFAULT '', | |
| 11 | creator_id INTEGER REFERENCES users(id) ON DELETE SET NULL, | |
| 12 | created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), | |
| 13 | updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), | |
| 14 | UNIQUE (repo_id, commit_sha, context) | |
| 15 | ); | |
| 16 | ||
| 17 | INSERT INTO commit_statuses (id, repo_id, commit_sha, context, state, description, target_url, creator_id, created_at, updated_at) | |
| 18 | SELECT id, repo_id, commit_sha, context, state, description, target_url, creator_id, created_at, updated_at | |
| 19 | FROM commit_statuses_old; | |
| 20 | ||
| 21 | DROP TABLE commit_statuses_old; | |
| 22 | ||
| 23 | CREATE INDEX commit_statuses_sha ON commit_statuses(repo_id, commit_sha); | |
internal/store/statuses.go +1 −1
| @@ -7,7 +7,7 @@ import ( | ||
| 7 | 7 | |
| 8 | 8 | type CommitStatus struct { |
| 9 | 9 | Context string |
| 10 | State string // pending | success | failure | error | |
| 10 | State string // pending | success | failure | error | skipped | |
| 11 | 11 | Description string |
| 12 | 12 | TargetURL string |
| 13 | 13 | Creator string |
internal/store/statuses_test.go +26
| @@ -57,3 +57,29 @@ func TestChecksForCommit(t *testing.T) { | ||
| 57 | 57 | } |
| 58 | 58 | } |
| 59 | 59 | } |
| 60 | ||
| 61 | // A job a path filter excluded records "skipped" rather than nothing at | |
| 62 | // all (#172). It must satisfy require_checks like any other green state, | |
| 63 | // without a failure or pending status elsewhere being masked by it. | |
| 64 | func TestCombinedStatusTreatsSkippedAsGreen(t *testing.T) { | |
| 65 | only := []CommitStatus{{Context: "ci/unit", State: "skipped"}} | |
| 66 | if got := CombinedStatus(only); got != "success" { | |
| 67 | t.Fatalf("all-skipped combined status: %s", got) | |
| 68 | } | |
| 69 | ||
| 70 | withPending := []CommitStatus{ | |
| 71 | {Context: "ci/unit", State: "skipped"}, | |
| 72 | {Context: "ci/lint", State: "pending"}, | |
| 73 | } | |
| 74 | if got := CombinedStatus(withPending); got != "pending" { | |
| 75 | t.Fatalf("skipped plus pending combined status: %s", got) | |
| 76 | } | |
| 77 | ||
| 78 | withFailure := []CommitStatus{ | |
| 79 | {Context: "ci/unit", State: "skipped"}, | |
| 80 | {Context: "ci/lint", State: "failure"}, | |
| 81 | } | |
| 82 | if got := CombinedStatus(withFailure); got != "failure" { | |
| 83 | t.Fatalf("skipped plus failure combined status: %s", got) | |
| 84 | } | |
| 85 | } | |
internal/web/static/style.css +1
| @@ -1053,6 +1053,7 @@ table.difftable td.ln a.cmt:focus { color: var(--accent); text-decoration: under | ||
| 1053 | 1053 | .badge.check-pending { --chip: var(--warn); } |
| 1054 | 1054 | .badge.check-failure, .badge.check-error { --chip: var(--bad); } |
| 1055 | 1055 | .badge.check-cancelled, .chip.check-cancelled { --chip: var(--neutral); } |
| 1056 | .badge.check-skipped, .chip.check-skipped { --chip: var(--neutral); } | |
| 1056 | 1057 | .badge.check-running, .chip.check-running { --chip: var(--warn); } |
| 1057 | 1058 | .chip.check-success { --chip: var(--ok); } |
| 1058 | 1059 | .chip.check-pending { --chip: var(--warn); } |