refactor(pihole): reduce cyclomatic complexity of TestProviderV6 (#5876)

* refactor(pihole): reduce cyclomatic complexity of TestProviderV6

* chore(pihole): increase coverage

* style: linting

* style: linting

* fix: remove coverage html

Author: AndrewCharlesHay

.gitignore

@@ -42,6 +42,7 @@ cscope.*
 
 # coverage output
 cover.out
+coverage.html
 *.coverprofile
 external-dns
 

.golangci.yml

@@ -41,7 +41,7 @@ linters:
         - name: confusing-naming
           disabled: true
     cyclop: # Lower cyclomatic complexity threshold after the max complexity is lowered
-      max-complexity: 43
+      max-complexity: 37 # Controller/execute.go:147:1: calculated cyclomatic complexity for function buildProvider is 37
     testifylint:
       # Enable all checkers (https://github.com/Antonboom/testifylint#checkers).
       # Default: false

provider/pihole/clientV6_test.go

@@ -92,6 +92,12 @@ func newTestServerV6(t *testing.T, hdlr http.HandlerFunc) *httptest.Server {
 	return svr
 }
 
+type errorTransportV6 struct{}
+
+func (t *errorTransportV6) RoundTrip(req *http.Request) (*http.Response, error) {
+	return nil, errors.New("network error")
+}
+
 func TestNewPiholeClientV6(t *testing.T) {
 	// Test correct error on no server provided
 	_, err := newPiholeClientV6(PiholeConfig{APIVersion: "6"})
@@ -117,15 +123,18 @@ func TestNewPiholeClientV6(t *testing.T) {
 	srvr := newTestServerV6(t, func(w http.ResponseWriter, r *http.Request) {
 		if r.URL.Path == "/api/auth" && r.Method == http.MethodPost {
 			var requestData map[string]string
-			json.NewDecoder(r.Body).Decode(&requestData)
+			err := json.NewDecoder(r.Body).Decode(&requestData)
+			if err != nil {
+				t.Fatal(err)
+			}
 			defer r.Body.Close()
 
 			w.Header().Set("Content-Type", "application/json")
 
 			if requestData["password"] != "correct" {
 				// Return unsuccessful authentication response
 				w.WriteHeader(http.StatusUnauthorized)
-				w.Write([]byte(`{
+				_, err = w.Write([]byte(`{
 				"session": {
 					"valid": false,
 					"totp": false,
@@ -135,11 +144,14 @@ func TestNewPiholeClientV6(t *testing.T) {
 				},
 				"took": 0.2
 			}`))
+				if err != nil {
+					t.Fatal(err)
+				}
 				return
 			}
 
 			// Return successful authentication response
-			w.Write([]byte(`{
+			_, err = w.Write([]byte(`{
 			"session": {
 				"valid": true,
 				"totp": false,
@@ -185,7 +197,7 @@ func TestListRecordsV6(t *testing.T) {
 			w.Header().Set("Content-Type", "application/json")
 
 			// Return A records
-			w.Write([]byte(`{
+			if _, err := w.Write([]byte(`{
 				"config": {
 					"dns": {
 						"hosts": [
@@ -205,7 +217,9 @@ func TestListRecordsV6(t *testing.T) {
 					}
 				},
 				"took": 5
-			}`))
+			}`)); err != nil {
+				t.Fatal(err)
+			}
 		} else if r.URL.Path == "/api/config/dns/cnameRecords" && r.Method == http.MethodGet {
 
 			w.WriteHeader(http.StatusOK)
@@ -384,9 +398,12 @@ func TestErrorsV6(t *testing.T) {
 		Server:     "not an url",
 		APIVersion: "6",
 	}
-	clErrURL, _ := newPiholeClientV6(cfgErrURL)
+	clErrURL, err := newPiholeClientV6(cfgErrURL)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	_, err := clErrURL.listRecords(context.Background(), endpoint.RecordTypeCNAME)
+	_, err = clErrURL.listRecords(context.Background(), endpoint.RecordTypeCNAME)
 	if err == nil {
 		t.Fatal("Expected error for using invalid URL")
 	}
@@ -785,6 +802,80 @@ func TestDoRetryOne(t *testing.T) {
 
 }
 
+func TestDoV6AdditionalCases(t *testing.T) {
+	t.Run("http client error", func(t *testing.T) {
+		client := &piholeClientV6{
+			httpClient: &http.Client{
+				Transport: &errorTransportV6{},
+			},
+		}
+		req, _ := http.NewRequest(http.MethodGet, "http://localhost", nil)
+		_, err := client.do(req)
+		if err == nil {
+			t.Fatal("expected an error, but got none")
+		}
+		if !strings.Contains(err.Error(), "network error") {
+			t.Fatalf("expected error to contain 'network error', but got '%v'", err)
+		}
+	})
+
+	t.Run("item already present", func(t *testing.T) {
+		server := newTestServerV6(t, func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusBadRequest)
+			w.Write([]byte(`{
+				"error": {
+					"key": "bad_request",
+					"message": "Item already present",
+					"hint": "The item you're trying to add already exists"
+				},
+				"took": 0.1
+			}`))
+		})
+		defer server.Close()
+
+		client := &piholeClientV6{
+			httpClient: server.Client(),
+			token:      "test-token",
+		}
+		req, _ := http.NewRequest(http.MethodPut, server.URL+"/api/test", nil)
+		resp, err := client.do(req)
+		if err != nil {
+			t.Fatalf("expected no error for 'Item already present', but got '%v'", err)
+		}
+		if resp == nil {
+			t.Fatal("expected response, but got nil")
+		}
+	})
+
+	t.Run("404 on DELETE", func(t *testing.T) {
+		server := newTestServerV6(t, func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusNotFound)
+			w.Write([]byte(`{
+				"error": {
+					"key": "not_found",
+					"message": "Item not found",
+					"hint": "The item you're trying to delete does not exist"
+				},
+				"took": 0.1
+			}`))
+		})
+		defer server.Close()
+
+		client := &piholeClientV6{
+			httpClient: server.Client(),
+			token:      "test-token",
+		}
+		req, _ := http.NewRequest(http.MethodDelete, server.URL+"/api/test", nil)
+		resp, err := client.do(req)
+		if err != nil {
+			t.Fatalf("expected no error for 404 on DELETE, but got '%v'", err)
+		}
+		if resp == nil {
+			t.Fatal("expected response, but got nil")
+		}
+	})
+}
+
 func TestCreateRecordV6(t *testing.T) {
 	var ep *endpoint.Endpoint
 	srvr := newTestServerV6(t, func(w http.ResponseWriter, r *http.Request) {

provider/pihole/client_test.go

@@ -57,22 +57,31 @@ func TestNewPiholeClient(t *testing.T) {
 
 	// Create a test server for auth tests
 	srvr := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {
-		r.ParseForm()
+		err := r.ParseForm()
+		if err != nil {
+			t.Fatal(err)
+		}
 		pw := r.Form.Get("pw")
 		if pw != "correct" {
 			// Pihole actually server side renders the fact that you failed, normal 200
-			w.Write([]byte("Invalid"))
+			_, err = w.Write([]byte("Invalid"))
+			if err != nil {
+				t.Fatal(err)
+			}
 			return
 		}
 		// This is a subset of what happens on successful login
-		w.Write([]byte(`
+		_, err = w.Write([]byte(`
 		<!doctype html>
 		<html lang="en">
 			<body>
 				<div id="token" hidden>supersecret</div>
 			</body>
 		</html>
 		`))
+		if err != nil {
+			t.Fatal(err)
+		}
 	})
 	defer srvr.Close()
 
@@ -124,12 +133,15 @@ func CheckRecordRetrieval(t *testing.T, cl *piholeClient, recordType string, exp
 
 func TestListRecords(t *testing.T) {
 	srvr := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {
-		r.ParseForm()
+		err := r.ParseForm()
+		if err != nil {
+			t.Fatal(err)
+		}
 		if r.Form.Get("action") != "get" {
 			t.Error("Expected 'get' action in form from client")
 		}
 		if strings.Contains(r.URL.Path, "cname") {
-			w.Write([]byte(`
+			_, err = w.Write([]byte(`
 			{
 				"data": [
 					["test4.example.com", "cname.example.com"],
@@ -138,10 +150,13 @@ func TestListRecords(t *testing.T) {
 				]
 			}
 			`))
+			if err != nil {
+				t.Fatal(err)
+			}
 			return
 		}
 		// Pihole makes no distinction between A and AAAA records
-		w.Write([]byte(`
+		_, err = w.Write([]byte(`
 		{
 			"data": [
 				["test1.example.com", "192.168.1.1"],
@@ -153,6 +168,9 @@ func TestListRecords(t *testing.T) {
 			]
 		}
 		`))
+		if err != nil {
+			t.Fatal(err)
+		}
 	})
 	defer srvr.Close()
 
@@ -243,41 +261,32 @@ func testErrorScenarios(t *testing.T, srvrErr *httptest.Server) {
 func TestErrorScenarios(t *testing.T) {
 	// Test errors token
 	srvrErr := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {
-		r.ParseForm()
+		err := r.ParseForm()
+		if err != nil {
+			t.Fatal(err)
+		}
 		pw := r.Form.Get("pw")
 		if pw != "" {
 			if pw != "correct" {
-				// Pihole actually server side renders the fact that you failed, normal 200
-				w.Write([]byte("Invalid"))
+				_, err = w.Write([]byte("Invalid"))
+				if err != nil {
+					t.Fatal(err)
+				}
 				return
 			}
 		}
 		if strings.Contains(r.URL.Path, "admin/scripts/pi-hole/php/customcname.php") && r.Form.Get("token") == "correct" {
-			w.Write([]byte(`
+			_, err = w.Write([]byte(`
 				{
 					"nodata": [
 						["nodata", "no"]
 					]
 				}
-				`))
-			return
-		}
-		if strings.Contains(r.URL.Path, "admin/index.php?login") {
-			w.Write([]byte(`
-			<!doctype html>
-			<html lang="en">
-				<body>
-					<div id="token" hidden>supersecret</div>
-				</body>
-			</html>
 			`))
+			if err != nil {
+				t.Fatal(err)
+			}
 		}
-		// Token Expired
-		w.Write([]byte(`
-		{
-			"auth": "expired"
-		}
-		`))
 	})
 	defer srvrErr.Close()