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

Fix StatusJSONResponse usage (#10810)

Browse source 
* Fix `StatusJSONResponse` usage

Signed-off-by: Babak K. Shandiz <babakks@github.com>

* Replace `assert` with `require`

Signed-off-by: Babak K. Shandiz <babakks@github.com>

* Improve assertion against errors

Signed-off-by: Babak K. Shandiz <babakks@github.com>

* Add `JSONErrorResponse` helper func

Signed-off-by: Babak K. Shandiz <babakks@github.com>

* Use `httpmock.JSONErrorResponse`

Signed-off-by: Babak K. Shandiz <babakks@github.com>

* Replace `StatusJSONResponse` to `JSONErrorResponse` for better readibility

Signed-off-by: Babak K. Shandiz <babakks@github.com>

* Fix improper use of `StatsJSONResponse`

Signed-off-by: Babak K. Shandiz <babakks@github.com>

---------

Signed-off-by: Babak K. Shandiz <babakks@github.com>
This commit is contained in:
Babak K. Shandiz 2025-05-01 20:22:43 +01:00 • committed by GitHub
parent 0a1e7a1fdc
commit 284880c21e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 52 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

50
pkg/cmd/gist/delete/delete_test.go
View file

@ -18,6 +18,7 @@ import (
ghAPI "github.com/cli/go-gh/v2/pkg/api"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNewCmdDelete(t *testing.T) {
@ -327,11 +328,12 @@ func Test_deleteRun(t *testing.T) {
func Test_gistDelete(t *testing.T) {
tests := []struct {
name string
httpStubs func(*httpmock.Registry)
hostname string
gistID string
wantErr error
name string
httpStubs func(*httpmock.Registry)
hostname string
gistID string
wantErr error
wantErrString string
}{
{
name: "successful delete",
@ -343,36 +345,34 @@ func Test_gistDelete(t *testing.T) {
},
hostname: "github.com",
gistID: "1234",
wantErr: nil,
},
{
name: "when an gist is not found, it returns a NotFoundError",
name: "when a gist is not found, it returns a NotFoundError",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("DELETE", "gists/1234"),
httpmock.StatusStringResponse(404, "{}"),
)
},
hostname: "github.com",
gistID: "1234",
wantErr: shared.NotFoundErr,
hostname: "github.com",
gistID: "1234",
wantErr: shared.NotFoundErr, // To make sure we return the pre-defined error instance.
wantErrString: "not found",
},
{
name: "when there is a non-404 error deleting the gist, that error is returned",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("DELETE", "gists/1234"),
httpmock.StatusJSONResponse(500, `{"message": "arbitrary error"}`),
httpmock.JSONErrorResponse(500, ghAPI.HTTPError{
StatusCode: 500,
Message: "arbitrary error",
}),
)
},
hostname: "github.com",
gistID: "1234",
wantErr: api.HTTPError{
HTTPError: &ghAPI.HTTPError{
StatusCode: 500,
Message: "arbitrary error",
},
},
hostname: "github.com",
gistID: "1234",
wantErrString: "HTTP 500: arbitrary error (https://api.github.com/gists/1234)",
},
}
@ -383,12 +383,16 @@ func Test_gistDelete(t *testing.T) {
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
err := deleteGist(client, tt.hostname, tt.gistID)
if tt.wantErr != nil {
assert.ErrorAs(t, err, &tt.wantErr)
if tt.wantErrString == "" && tt.wantErr == nil {
require.NoError(t, err)
} else {
assert.NoError(t, err)
if tt.wantErrString != "" {
require.EqualError(t, err, tt.wantErrString)
}
if tt.wantErr != nil {
require.ErrorIs(t, err, tt.wantErr)
}
}
})
}
}

2
pkg/cmd/gpg-key/delete/delete_test.go
View file

@ -177,7 +177,7 @@ func Test_deleteRun(t *testing.T) {
opts: DeleteOptions{KeyID: "ABC123", Confirmed: true},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "user/gpg_keys"), httpmock.StatusStringResponse(200, keysResp))
reg.Register(httpmock.REST("DELETE", "user/gpg_keys/123"), httpmock.StatusJSONResponse(404, api.HTTPError{
reg.Register(httpmock.REST("DELETE", "user/gpg_keys/123"), httpmock.JSONErrorResponse(404, api.HTTPError{
StatusCode: 404,
Message: "GPG key 123 not found",
}))

22
pkg/cmd/repo/autolink/delete/http_test.go
View file

@ -7,6 +7,7 @@ import (
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/go-gh/v2/pkg/api"
"github.com/stretchr/testify/require"
)
@ -14,10 +15,10 @@ func TestAutolinkDeleter_Delete(t *testing.T) {
repo := ghrepo.New("OWNER", "REPO")
tests := []struct {
name string
id string
stubStatus int
stubRespJSON string
name string
id string
stubStatus int
stubResp any
expectErr bool
expectedErrMsg string
@ -31,17 +32,18 @@ func TestAutolinkDeleter_Delete(t *testing.T) {
name: "404 repo or autolink not found",
id: "123",
stubStatus: http.StatusNotFound,
stubRespJSON: `{}`, // API response not used in output
expectErr: true,
expectedErrMsg: "error deleting autolink: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks/123)",
},
{
name: "500 unexpected error",
id: "123",
stubRespJSON: `{"messsage": "arbitrary error"}`,
name: "500 unexpected error",
id: "123",
stubResp: api.HTTPError{
Message: "arbitrary error",
},
stubStatus: http.StatusInternalServerError,
expectErr: true,
expectedErrMsg: "HTTP 500 (https://api.github.com/repos/OWNER/REPO/autolinks/123)",
expectedErrMsg: "HTTP 500: arbitrary error (https://api.github.com/repos/OWNER/REPO/autolinks/123)",
},
}
@ -53,7 +55,7 @@ func TestAutolinkDeleter_Delete(t *testing.T) {
http.MethodDelete,
fmt.Sprintf("repos/%s/%s/autolinks/%s", repo.RepoOwner(), repo.RepoName(), tt.id),
),
httpmock.StatusJSONResponse(tt.stubStatus, tt.stubRespJSON),
httpmock.StatusJSONResponse(tt.stubStatus, tt.stubResp),
)
defer reg.Verify(t)

2
pkg/cmd/run/watch/watch_test.go
View file

@ -316,7 +316,7 @@ func TestWatchRun(t *testing.T) {
)
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"),
httpmock.StatusJSONResponse(404, api.HTTPError{
httpmock.JSONErrorResponse(404, api.HTTPError{
StatusCode: 404,
Message: "run 1234 not found",
}),

11
pkg/httpmock/stub.go
View file

@ -9,6 +9,8 @@ import (
"os"
"regexp"
"strings"
"github.com/cli/go-gh/v2/pkg/api"
)
type Matcher func(req *http.Request) bool
@ -161,6 +163,9 @@ func JSONResponse(body interface{}) Responder {
}
}
// StatusJSONResponse turns the given argument into a JSON response.
//
// The argument is not meant to be a JSON string, unless it's intentional.
func StatusJSONResponse(status int, body interface{}) Responder {
return func(req *http.Request) (*http.Response, error) {
b, _ := json.Marshal(body)
@ -171,6 +176,12 @@ func StatusJSONResponse(status int, body interface{}) Responder {
}
}
// JSONErrorResponse is a type-safe helper to avoid confusion around the
// provided argument.
func JSONErrorResponse(status int, err api.HTTPError) Responder {
return StatusJSONResponse(status, err)
}
func FileResponse(filename string) Responder {
return func(req *http.Request) (*http.Response, error) {
f, err := os.Open(filename)