fix: address failing tests and linter errors

Author: RoseSecurity

forwardemail/aliases_test.go

@@ -4,6 +4,7 @@
 package forwardemail
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -282,7 +283,7 @@ func TestClient_CreateAlias(t *testing.T) {
 				"labels": [
 				  "catch-all"
 				],
-				Description: "main email",
+				"description": "main email",
 				"is_enabled": true,
 				"has_recipient_verification": true,
 				"recipients": [
@@ -381,7 +382,7 @@ func TestClient_UpdateAlias(t *testing.T) {
 				  "catch-all",
 				  "friends"
 				],
-				Description: "main email",
+				"description": "main email",
 				"is_enabled": true,
 				"has_recipient_verification": true,
 				"recipients": [
@@ -447,10 +448,10 @@ func TestClient_DeleteAlias(t *testing.T) {
 	}
 
 	tests := []struct {
-		name string
-		req  request
-		res  response
-		want error
+		name      string
+		req       request
+		res       response
+		wantError bool
 	}{
 		{
 			name: "ok",
@@ -464,15 +465,15 @@ func TestClient_DeleteAlias(t *testing.T) {
 				code: http.StatusInternalServerError,
 				body: "oh no",
 			},
-			want: fmt.Errorf("failed to complete request: oh no"), //nolint:all
+			wantError: true,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 				w.WriteHeader(tt.res.code)
-				fmt.Fprint(w, tt.res.body)
+				_, _ = w.Write([]byte(tt.res.body))
 			}))
 			defer svr.Close()
 
@@ -482,8 +483,15 @@ func TestClient_DeleteAlias(t *testing.T) {
 			})
 
 			got := c.DeleteAlias(tt.req.domain, tt.req.alias)
-			if diff := cmp.Diff(tt.want, got, cmp.Comparer(equateErrorMessage)); diff != "" {
-				t.Fatalf("values are not the same %s", diff)
+			if tt.wantError {
+				if got == nil {
+					t.Fatal("expected error, got nil")
+				}
+				if !errors.Is(got, ErrRequestFailure) {
+					t.Fatalf("expected error to wrap ErrRequestFailure, got %v", got)
+				}
+			} else if got != nil {
+				t.Fatalf("expected no error, got %v", got)
 			}
 		})
 	}

forwardemail/client_test.go

@@ -4,25 +4,26 @@
 package forwardemail
 
 import (
+	"errors"
 	"net/http"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 )
 
 func TestNewClient(t *testing.T) {
 	tests := []struct {
-		name    string
-		options ClientOptions
-		want    *Client
+		name      string
+		options   ClientOptions
+		want      *Client
+		wantError error
 	}{
 		{
-			name:    "empty options",
-			options: ClientOptions{},
-			want: &Client{
-				APIURL:     "https://api.forwardemail.net",
-				HTTPClient: &http.Client{},
-			},
+			name:      "empty options returns error",
+			options:   ClientOptions{},
+			want:      nil,
+			wantError: ErrMissingAPIKey,
 		},
 		{
 			name: "with api key",
@@ -32,18 +33,16 @@ func TestNewClient(t *testing.T) {
 			want: &Client{
 				APIKey:     "4e4d6c332b6fe62a63afe56171fd3725",
 				APIURL:     "https://api.forwardemail.net",
-				HTTPClient: &http.Client{},
+				HTTPClient: &http.Client{Timeout: 30 * time.Second},
 			},
 		},
 		{
-			name: "with api url",
+			name: "with api url but no api key returns error",
 			options: ClientOptions{
 				APIURL: "https://google.com",
 			},
-			want: &Client{
-				APIURL:     "https://google.com",
-				HTTPClient: &http.Client{},
-			},
+			want:      nil,
+			wantError: ErrMissingAPIKey,
 		},
 		{
 			name: "with everything at once",
@@ -54,14 +53,23 @@ func TestNewClient(t *testing.T) {
 			want: &Client{
 				APIKey:     "4e4d6c332b6fe62a63afe56171fd3725",
 				APIURL:     "https://google.com",
-				HTTPClient: &http.Client{},
+				HTTPClient: &http.Client{Timeout: 30 * time.Second},
 			},
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, _ := NewClient(tt.options)
+			got, err := NewClient(tt.options)
+			if tt.wantError != nil {
+				if !errors.Is(err, tt.wantError) {
+					t.Fatalf("expected error %v, got %v", tt.wantError, err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
 			if diff := cmp.Diff(tt.want, got); diff != "" {
 				t.Fatalf("values are not the same %s", diff)
 			}

forwardemail/domains_test.go

@@ -382,10 +382,10 @@ func TestClient_DeleteDomain(t *testing.T) {
 	}
 
 	tests := []struct {
-		name   string
-		domain string
-		resp   response
-		want   error
+		name      string
+		domain    string
+		resp      response
+		wantError bool
 	}{
 		{
 			name: "ok",
@@ -399,15 +399,15 @@ func TestClient_DeleteDomain(t *testing.T) {
 				code: http.StatusInternalServerError,
 				body: "oh no",
 			},
-			want: fmt.Errorf("failed to complete request: oh no"), //nolint:all
+			wantError: true,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 				w.WriteHeader(tt.resp.code)
-				fmt.Fprint(w, tt.resp.body)
+				_, _ = w.Write([]byte(tt.resp.body))
 			}))
 			defer svr.Close()
 
@@ -417,22 +417,20 @@ func TestClient_DeleteDomain(t *testing.T) {
 			})
 
 			got := c.DeleteDomain(tt.domain)
-			if diff := cmp.Diff(tt.want, got, cmp.Comparer(equateErrorMessage)); diff != "" {
-				t.Fatalf("values are not the same %s", diff)
+			if tt.wantError {
+				if got == nil {
+					t.Fatal("expected error, got nil")
+				}
+				if !errors.Is(got, ErrRequestFailure) {
+					t.Fatalf("expected error to wrap ErrRequestFailure, got %v", got)
+				}
+			} else if got != nil {
+				t.Fatalf("expected no error, got %v", got)
 			}
 		})
 	}
 }
 
-// I took this black magic from here (thanks Joe):
-// https://github.com/google/go-cmp/issues/24#issuecomment-317635190
-func equateErrorMessage(a, b error) bool {
-	if a == nil || b == nil {
-		return errors.Is(a, b)
-	}
-	return a.Error() == b.Error()
-}
-
 func pointBool(b bool) *bool {
 	return &b
 }