Commit 03620cf7bd
Verified · cmc ci/build: success ci/sonar: success ci/test: success ci/vuln: success
Layout: unified · split
internal/control/build.go +14
| @@ -633,6 +633,20 @@ func queueJobs( | |||
| 633 | continue | 633 | continue |
| 634 | } | 634 | } |
| 635 | diffOld := old | 635 | diffOld := old |
| 636 | // A force-push rewrote the branch, so the old tip is not an | ||
| 637 | // ancestor of the new one and old..new is not "what this push | ||
| 638 | // changed" — it is the difference between two histories. After a | ||
| 639 | // rebase that is whatever the new base added, typically nothing | ||
| 640 | // the branch itself touched, so every path filter concludes its | ||
| 641 | // job is unnecessary and the branch reads as green without its | ||
| 642 | // suite having run (#176). The merge base is the honest base: | ||
| 643 | // the filter is deciding about the branch's relationship to its | ||
| 644 | // target, which is what the merge base expresses. | ||
| 645 | if ci.HasDiffBase(diffOld) && deriveMergeBase { | ||
| 646 | if ok, err := gitutil.IsAncestor(dir, diffOld, sha); err != nil || !ok { | ||
| 647 | diffOld = "" | ||
| 648 | } | ||
| 649 | } | ||
| 636 | if !ci.HasDiffBase(diffOld) && deriveMergeBase { | 650 | if !ci.HasDiffBase(diffOld) && deriveMergeBase { |
| 637 | if base, err := gitutil.MergeBase(dir, "refs/heads/"+repo.DefaultBranch, sha); err == nil && base != sha { | 651 | if base, err := gitutil.MergeBase(dir, "refs/heads/"+repo.DefaultBranch, sha); err == nil && base != sha { |
| 638 | diffOld = base | 652 | diffOld = base |
internal/control/build_test.go +60
| @@ -489,3 +489,63 @@ func TestQueueMRBuildsStillFailsOpen(t *testing.T) { | |||
| 489 | t.Fatalf("expected the MR head to fail open and queue a build: %v %v", builds, err) | 489 | t.Fatalf("expected the MR head to fail open and queue a build: %v %v", builds, err) |
| 490 | } | 490 | } |
| 491 | } | 491 | } |
| 492 | |||
| 493 | // A force-push that rewrote the branch must not filter against the old | ||
| 494 | // tip. After a rebase, old..new is the difference between two histories | ||
| 495 | // — whatever the new base added — so a branch whose own commits touch | ||
| 496 | // code looks like a docs-only push and its jobs are skipped. It then | ||
| 497 | // reads as green without having run (#176). | ||
| 498 | func TestQueueBranchBuildsRebaseFiltersAgainstMergeBase(t *testing.T) { | ||
| 499 | st, repo, uid := newQueueTestRepo(t) | ||
| 500 | git := gitRunner(t) | ||
| 501 | root := t.TempDir() | ||
| 502 | |||
| 503 | src := filepath.Join(root, "src") | ||
| 504 | os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755) | ||
| 505 | os.MkdirAll(filepath.Join(src, "docs"), 0o755) | ||
| 506 | os.MkdirAll(filepath.Join(src, "app"), 0o755) | ||
| 507 | os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte( | ||
| 508 | "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo hi\n"), 0o644) | ||
| 509 | os.WriteFile(filepath.Join(src, "app", "a.go"), []byte("package a\n"), 0o644) | ||
| 510 | git(root, "init", "-q", "-b", "main", "src") | ||
| 511 | git(src, "add", ".") | ||
| 512 | git(src, "commit", "-q", "-m", "base") | ||
| 513 | |||
| 514 | // A branch that changes code — the kind of change the filter must | ||
| 515 | // never skip. | ||
| 516 | git(src, "checkout", "-q", "-b", "feat") | ||
| 517 | os.WriteFile(filepath.Join(src, "app", "a.go"), []byte("package a\n\nvar X = 1\n"), 0o644) | ||
| 518 | git(src, "add", ".") | ||
| 519 | git(src, "commit", "-q", "-m", "code change") | ||
| 520 | oldTip := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | ||
| 521 | |||
| 522 | // main moves on with a docs-only commit, and the branch is rebased | ||
| 523 | // onto it — exactly what a fast-forward-only repository forces. | ||
| 524 | git(src, "checkout", "-q", "main") | ||
| 525 | os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x\n"), 0o644) | ||
| 526 | git(src, "add", ".") | ||
| 527 | git(src, "commit", "-q", "-m", "docs only") | ||
| 528 | git(src, "checkout", "-q", "feat") | ||
| 529 | git(src, "rebase", "-q", "main") | ||
| 530 | newTip := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | ||
| 531 | |||
| 532 | // The trap: between the two tips lies only the docs commit. | ||
| 533 | if diff := git(src, "diff", "--name-only", oldTip, newTip); !strings.Contains(diff, "docs/x.md") || | ||
| 534 | strings.Contains(diff, "app/a.go") { | ||
| 535 | t.Fatalf("fixture does not reproduce the trap; old..new = %q", diff) | ||
| 536 | } | ||
| 537 | |||
| 538 | dir := RepoDir(root, repo.OwnerName, repo.Name) | ||
| 539 | os.MkdirAll(filepath.Dir(dir), 0o755) | ||
| 540 | git(root, "clone", "-q", "--bare", src, dir) | ||
| 541 | |||
| 542 | QueueBranchBuilds(st, root, "https://x.test", repo, uid, "feat", oldTip, newTip, time.Now()) | ||
| 543 | |||
| 544 | builds, err := st.ListBuilds(repo.ID, 10) | ||
| 545 | if err != nil { | ||
| 546 | t.Fatal(err) | ||
| 547 | } | ||
| 548 | if len(builds) == 0 { | ||
| 549 | t.Fatal("a rebased branch whose commits change code queued no build") | ||
| 550 | } | ||
| 551 | } | ||