cli · babakks · May 1, 2025 · Apr 16, 2025 · Apr 25, 2025 · Apr 25, 2025
@@ -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)
+				}
 			}
-
 		})
 	}
 }
@@ -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",
 				}))

@@ -7,17 +7,18 @@ 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"
 )
 
 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)
 

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

@@ -9,6 +9,8 @@ import (
 	"os"
 	"regexp"
 	"strings"
+
+	"github.com/cli/go-gh/v2/pkg/api"
 )
 
 type Matcher func(req *http.Request) bool
@@ -160,6 +162,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)
@@ -170,6 +175,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)