internal/task: remove DeleteBranch support in FakeGerrit For golang/go#81171 Change-Id: I8d08649c858fc08b9474638b48a413c9d4512504 Reviewed-on: https://go-review.googlesource.com/c/build/+/823286 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Neal Patel <nealpatel@google.com> Reviewed-by: David Chase <drchase@google.com>
diff --git a/internal/task/fakes.go b/internal/task/fakes.go index 92ad090..d4eab18 100644 --- a/internal/task/fakes.go +++ b/internal/task/fakes.go
@@ -376,33 +376,6 @@ return g.ReadBranchHead(ctx, project, branch) } -func (g *FakeGerrit) DeleteBranch(ctx context.Context, project, branch string) error { - repo, err := g.repo(project) - if err != nil { - return err - } - if _, err := repo.dir.RunCommand(ctx, "rev-parse", "--verify", "refs/heads/"+branch); err != nil { - if strings.Contains(err.Error(), "Needed a single revision") { - // Real Gerrit reports a DELETE on a branch that doesn't exist as a - // 404 Not Found, not an underlying git error. - return NewGerritHTTPError(http.StatusNotFound, fmt.Sprintf("branch %q not found\n", "refs/heads/"+branch)) - } - return err - } - g.changesMu.Lock() - for id, ci := range g.cls { - if ci.Status != gerrit.ChangeStatusAbandoned && ci.Status != gerrit.ChangeStatusMerged && ci.Branch == branch && g.clProjects[id] == project { - g.changesMu.Unlock() - return NewGerritHTTPError(http.StatusConflict, fmt.Sprintf("branch %q has open changes\n", "refs/heads/"+branch)) - } - } - g.changesMu.Unlock() - if _, err = repo.dir.RunCommand(ctx, "branch", "-D", branch); err != nil { - return err - } - return nil -} - func (g *FakeGerrit) ReadFile(ctx context.Context, project, commit, file string) ([]byte, error) { repo, err := g.repo(project) if err != nil {
diff --git a/internal/task/gerrit.go b/internal/task/gerrit.go index 7256440..db2d947 100644 --- a/internal/task/gerrit.go +++ b/internal/task/gerrit.go
@@ -73,11 +73,6 @@ ListBranches(ctx context.Context, project string) ([]gerrit.BranchInfo, error) // CreateBranch creates the given branch and returns the created branch's revision. CreateBranch(ctx context.Context, project, branch string, input gerrit.BranchInput) (string, error) - // DeleteBranch deletes the given branch. - // Deleting a branch that doesn't exist returns an error satisfying - // [errors.Is](err, [gerrit.ErrResourceNotExist]); callers that delete - // unconditionally (e.g. delete-then-recreate) must tolerate that error. - DeleteBranch(ctx context.Context, project, branch string) error // ListCommits lists commits in project between head and base (including head, not including base). // Both head and base must be non-empty strings, otherwise an error is returned. @@ -341,10 +336,6 @@ return branchInfo.Revision, nil } -func (c *RealGerritClient) DeleteBranch(ctx context.Context, project, branch string) error { - return c.Client.DeleteBranch(ctx, project, branch) -} - func (c *RealGerritClient) ListProjects(ctx context.Context) ([]string, error) { projects, err := c.Client.ListProjects(ctx) if err != nil {
diff --git a/internal/task/gerrit_test.go b/internal/task/gerrit_test.go index 7a9a782..9b06d91 100644 --- a/internal/task/gerrit_test.go +++ b/internal/task/gerrit_test.go
@@ -6,47 +6,12 @@ import ( "context" - "errors" "testing" "golang.org/x/build/gerrit" wf "golang.org/x/build/internal/workflow" ) -func TestFakeGerritDeleteBranch(t *testing.T) { - ctx := context.Background() - repo := NewFakeRepo(t, "deletebranch") - gc := NewFakeGerrit(t, repo) - - head, err := gc.ReadBranchHead(ctx, "deletebranch", "master") - if err != nil { - t.Fatalf("ReadBranchHead(master) failed: %v", err) - } - - // Create a branch, then confirm it exists. - if _, err := gc.CreateBranch(ctx, "deletebranch", "topic", gerrit.BranchInput{Revision: head}); err != nil { - t.Fatalf("CreateBranch(topic) failed: %v", err) - } - if _, err := gc.ReadBranchHead(ctx, "deletebranch", "topic"); err != nil { - t.Fatalf("ReadBranchHead(topic) after create failed: %v", err) - } - - // Delete the branch, then confirm it's gone. - if err := gc.DeleteBranch(ctx, "deletebranch", "topic"); err != nil { - t.Fatalf("DeleteBranch(topic) failed: %v", err) - } - if _, err := gc.ReadBranchHead(ctx, "deletebranch", "topic"); !errors.Is(err, gerrit.ErrResourceNotExist) { - t.Fatalf("ReadBranchHead(topic) after delete = %v, want error satisfying gerrit.ErrResourceNotExist", err) - } - - // Deleting a branch that doesn't exist mirrors real Gerrit's 404, - // returning an error satisfying gerrit.ErrResourceNotExist so that - // delete-then-recreate callers can delete unconditionally. - if err := gc.DeleteBranch(ctx, "deletebranch", "topic"); !errors.Is(err, gerrit.ErrResourceNotExist) { - t.Fatalf("DeleteBranch(topic) on missing branch = %v, want error satisfying gerrit.ErrResourceNotExist", err) - } -} - func TestNoOpCL(t *testing.T) { if !*flagRunVersionTest { t.Skip("Not enabled by flags")
diff --git a/internal/task/privx_test.go b/internal/task/privx_test.go index abfca28..9b7eb17 100644 --- a/internal/task/privx_test.go +++ b/internal/task/privx_test.go
@@ -845,34 +845,6 @@ } } -func TestDeleteBranch409Refusal(t *testing.T) { - repo := NewFakeRepo(t, "test") - head := repo.History()[0] - repo.Branch("my-branch", head) - - fg := NewFakeGerrit(t, repo) - fg.AddChange("test", "open-cl", &gerrit.ChangeInfo{ - ID: "open-cl", - ChangeID: "open-cl", - Branch: "my-branch", - Status: "NEW", - Submittable: true, - }, "open change") - - ctx := context.Background() - err := fg.DeleteBranch(ctx, "test", "my-branch") - if err == nil { - t.Fatal("expected 409 error for branch with open CLs") - } - var httpErr *gerrit.HTTPError - if !errors.As(err, &httpErr) { - t.Fatalf("error type = %T, want *gerrit.HTTPError", err) - } - if httpErr.Res.StatusCode != http.StatusConflict { - t.Errorf("status = %d, want %d", httpErr.Res.StatusCode, http.StatusConflict) - } -} - func TestCreateCherryPickChangeIDUniqueness(t *testing.T) { repo := NewFakeRepo(t, "test") head := repo.History()[0]