STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #6260 from cli/fix-branch-delete

Browse source 
Fix deleting remote branches with `#` in their name
This commit is contained in:
Mislav Marohnić 2022-09-15 11:06:04 +02:00 • committed by GitHub
commit cfb7e66d2b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 38 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

3
api/queries_pr.go
View file

@ -3,6 +3,7 @@ package api
import (
"fmt"
"net/http"
"net/url"
"time"
"github.com/cli/cli/v2/internal/ghrepo"
@ -456,6 +457,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)
}

40
api/queries_pr_test.go
View file

@ -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)
}

8
pkg/cmd/workflow/disable/disable_test.go
View file

@ -133,7 +133,7 @@ func TestDisableRun(t *testing.T) {
tty: true,
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),
@ -158,7 +158,7 @@ func TestDisableRun(t *testing.T) {
tty: true,
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/another workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/another%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),
@ -217,7 +217,7 @@ func TestDisableRun(t *testing.T) {
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),
@ -242,7 +242,7 @@ func TestDisableRun(t *testing.T) {
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/another workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/another%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),

12
pkg/cmd/workflow/enable/enable_test.go
View file

@ -133,7 +133,7 @@ func TestEnableRun(t *testing.T) {
tty: true,
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/terrible workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/terrible%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),
@ -159,7 +159,7 @@ func TestEnableRun(t *testing.T) {
tty: true,
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a disabled workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a%20disabled%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),
@ -188,7 +188,7 @@ func TestEnableRun(t *testing.T) {
tty: true,
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a disabled inactivity workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a%20disabled%20inactivity%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),
@ -243,7 +243,7 @@ func TestEnableRun(t *testing.T) {
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/terrible workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/terrible%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),
@ -268,7 +268,7 @@ func TestEnableRun(t *testing.T) {
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a disabled inactivity workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a%20disabled%20inactivity%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),
@ -292,7 +292,7 @@ func TestEnableRun(t *testing.T) {
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a disabled workflow"),
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/a%20disabled%20workflow"),
httpmock.StatusStringResponse(404, "not found"))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"),

2
pkg/cmd/workflow/shared/shared.go
View file

@ -139,7 +139,7 @@ func getWorkflowByID(client *api.Client, repo ghrepo.Interface, ID string) (*Wor
var workflow Workflow
err := client.REST(repo.RepoHost(), "GET",
fmt.Sprintf("repos/%s/actions/workflows/%s", ghrepo.FullName(repo), ID),
fmt.Sprintf("repos/%s/actions/workflows/%s", ghrepo.FullName(repo), url.PathEscape(ID)),
nil, &workflow)
if err != nil {

2
pkg/httpmock/stub.go
View file

@ -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
}
}