Fix deleting remote branches with # in their name

api/queries_pr.go

@@ -3,6 +3,7 @@ package api
 import (
 	"fmt"
 	"net/http"
+	"net/url"
 	"time"
 
 	"github.com/cli/cli/v2/internal/ghrepo"
@@ -447,6 +448,6 @@ func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) er
 }
 
 func BranchDeleteRemote(client *Client, repo ghrepo.Interface, branch string) error {
-	path := fmt.Sprintf("repos/%s/%s/git/refs/heads/%s", repo.RepoOwner(), repo.RepoName(), branch)
+	path := fmt.Sprintf("repos/%s/%s/git/refs/heads/%s", repo.RepoOwner(), repo.RepoName(), url.PathEscape(branch))
 	return client.REST(repo.RepoHost(), "DELETE", path, nil, nil)
 }

api/queries_pr_test.go

@@ -11,36 +11,44 @@ import (
 
 func TestBranchDeleteRemote(t *testing.T) {
 	var tests = []struct {
-		name           string
-		responseStatus int
-		responseBody   string
-		expectError    bool
+		name        string
+		branch      string
+		httpStubs   func(*httpmock.Registry)
+		expectError bool
 	}{
 		{
-			name:           "success",
-			responseStatus: 204,
-			responseBody:   "",
-			expectError:    false,
+			name:   "success",
+			branch: "owner/branch#123",
+			httpStubs: func(reg *httpmock.Registry) {
+				reg.Register(
+					httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/owner%2Fbranch%23123"),
+					httpmock.StatusStringResponse(204, ""))
+			},
+			expectError: false,
 		},
 		{
-			name:           "error",
-			responseStatus: 500,
-			responseBody:   `{"message": "oh no"}`,
-			expectError:    true,
+			name:   "error",
+			branch: "my-branch",
+			httpStubs: func(reg *httpmock.Registry) {
+				reg.Register(
+					httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/my-branch"),
+					httpmock.StatusStringResponse(500, `{"message": "oh no"}`))
+			},
+			expectError: true,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			http := &httpmock.Registry{}
-			http.Register(
-				httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/branch"),
-				httpmock.StatusStringResponse(tt.responseStatus, tt.responseBody))
+			if tt.httpStubs != nil {
+				tt.httpStubs(http)
+			}
 
 			client := newTestClient(http)
 			repo, _ := ghrepo.FromFullName("OWNER/REPO")
 
-			err := BranchDeleteRemote(client, repo, "branch")
+			err := BranchDeleteRemote(client, repo, tt.branch)
 			if (err != nil) != tt.expectError {
 				t.Fatalf("unexpected result: %v", err)
 			}

pkg/httpmock/stub.go

@@ -29,7 +29,7 @@ func REST(method, p string) Matcher {
 		if !strings.EqualFold(req.Method, method) {
 			return false
 		}
-		return req.URL.Path == "/"+p
+		return req.URL.EscapedPath() == "/"+p
 	}
 }