chore: Simplify form value assertions in tests (#4048)

Author: alexandear

github/github_test.go

@@ -119,16 +119,30 @@ func testMethod(t *testing.T, r *http.Request, want string) {
 
 type values map[string]string
 
-func testFormValues(t *testing.T, r *http.Request, values values) {
+// testFormValues checks that the request form values match the expected values.
+// It expects exactly one value per key.
+func testFormValues(t *testing.T, r *http.Request, want values) {
 	t.Helper()
-	want := url.Values{}
-	for k, v := range values {
-		want.Set(k, v)
+
+	wantValues := url.Values{}
+	for k, v := range want {
+		wantValues.Add(k, v)
+	}
+
+	assertNilError(t, r.ParseForm())
+	if got := r.Form; !cmp.Equal(got, wantValues) {
+		t.Errorf("Request query parameters: %v, want %v", got, wantValues)
 	}
+}
+
+// testFormValuesList checks that the request form values match the expected values.
+// It allows for multiple values per key.
+func testFormValuesList(t *testing.T, r *http.Request, want url.Values) {
+	t.Helper()
 
 	assertNilError(t, r.ParseForm())
 	if got := r.Form; !cmp.Equal(got, want) {
-		t.Errorf("Request parameters: %v, want %v", got, want)
+		t.Errorf("Request query parameters: %v, want %v", got, want)
 	}
 }
 

github/orgs_audit_log_test.go

@@ -8,7 +8,6 @@ package github
 import (
 	"fmt"
 	"net/http"
-	"strings"
 	"testing"
 	"time"
 )
@@ -19,6 +18,7 @@ func TestOrganizationService_GetAuditLog(t *testing.T) {
 
 	mux.HandleFunc("/orgs/o/audit-log", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
+		testFormValues(t, r, values{"include": "all", "phrase": "action:workflows", "order": "asc"})
 
 		fmt.Fprint(w, `[
 	{
@@ -86,7 +86,7 @@ func TestOrganizationService_GetAuditLog(t *testing.T) {
 		Order:   Ptr("asc"),
 	}
 
-	auditEntries, resp, err := client.Organizations.GetAuditLog(ctx, "o", &getOpts)
+	auditEntries, _, err := client.Organizations.GetAuditLog(ctx, "o", &getOpts)
 	if err != nil {
 		t.Errorf("Organizations.GetAuditLog returned error: %v", err)
 	}
@@ -151,12 +151,6 @@ func TestOrganizationService_GetAuditLog(t *testing.T) {
 
 	assertNoDiff(t, want, auditEntries)
 
-	// assert query string has lower case params
-	requestedQuery := resp.Request.URL.RawQuery
-	if !strings.Contains(requestedQuery, "phrase") {
-		t.Errorf("Organizations.GetAuditLog query string \ngot: %+v,\nwant:%+v", requestedQuery, "phrase")
-	}
-
 	const methodName = "GetAuditLog"
 	testBadOptions(t, methodName, func() (err error) {
 		_, _, err = client.Organizations.GetAuditLog(ctx, "\n", &getOpts)

github/orgs_personal_access_tokens_test.go

@@ -9,7 +9,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"strings"
+	"net/url"
 	"testing"
 	"time"
 
@@ -22,26 +22,13 @@ func TestOrganizationsService_ListFineGrainedPersonalAccessTokens(t *testing.T)
 
 	mux.HandleFunc("/orgs/o/personal-access-tokens", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		expectedQuery := map[string][]string{
+		testFormValuesList(t, r, url.Values{
 			"per_page":  {"2"},
 			"page":      {"2"},
 			"sort":      {"created_at"},
 			"direction": {"desc"},
 			"owner[]":   {"octocat", "octodog", "otherbot"},
-		}
-
-		query := r.URL.Query()
-		for key, expectedValues := range expectedQuery {
-			actualValues := query[key]
-			if len(actualValues) != len(expectedValues) {
-				t.Errorf("Expected %v values for query param %v, got %v", len(expectedValues), key, len(actualValues))
-			}
-			for i, expectedValue := range expectedValues {
-				if actualValues[i] != expectedValue {
-					t.Errorf("Expected query param %v to be %v, got %v", key, expectedValue, actualValues[i])
-				}
-			}
-		}
+		})
 
 		fmt.Fprint(w, `
 		[
@@ -165,13 +152,8 @@ func TestOrganizationsService_ListFineGrainedPersonalAccessTokens_ownerOnly(t *t
 
 	mux.HandleFunc("/orgs/o/personal-access-tokens", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		// When only Owner is set, addOptions adds no query params, so URL gets "?" + owner[]=...
-		if !strings.Contains(r.URL.RawQuery, "owner[]=") {
-			t.Errorf("Expected query to contain owner[]=, got %q", r.URL.RawQuery)
-		}
-		if strings.HasPrefix(r.URL.RawQuery, "&") {
-			t.Errorf("Expected query to start with ?, got %q", r.URL.RawQuery)
-		}
+		testFormValues(t, r, values{"owner[]": "octocat"})
+
 		fmt.Fprint(w, "[]")
 	})
 
@@ -189,27 +171,14 @@ func TestOrganizationsService_ListFineGrainedPersonalAccessTokenRequests(t *test
 
 	mux.HandleFunc("/orgs/o/personal-access-token-requests", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		expectedQuery := map[string][]string{
+		testFormValuesList(t, r, url.Values{
 			"per_page":   {"2"},
 			"page":       {"2"},
 			"sort":       {"created_at"},
 			"direction":  {"desc"},
 			"owner[]":    {"octocat", "octodog"},
 			"token_id[]": {"11579703", "11579704"},
-		}
-
-		query := r.URL.Query()
-		for key, expectedValues := range expectedQuery {
-			actualValues := query[key]
-			if len(actualValues) != len(expectedValues) {
-				t.Errorf("Expected %v values for query param %v, got %v", len(expectedValues), key, len(actualValues))
-			}
-			for i, expectedValue := range expectedValues {
-				if actualValues[i] != expectedValue {
-					t.Errorf("Expected query param %v to be %v, got %v", key, expectedValue, actualValues[i])
-				}
-			}
-		}
+		})
 
 		fmt.Fprint(w, `[
 			{
@@ -331,13 +300,7 @@ func TestOrganizationsService_ListFineGrainedPersonalAccessTokenRequests_ownerOn
 
 	mux.HandleFunc("/orgs/o/personal-access-token-requests", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		// When only Owner is set (no ListOptions, TokenID, etc.), addOptions adds no query params, so URL gets "?" + owner[]=...
-		if !strings.Contains(r.URL.RawQuery, "owner[]=") {
-			t.Errorf("Expected query to contain owner[]=, got %q", r.URL.RawQuery)
-		}
-		if strings.HasPrefix(r.URL.RawQuery, "&") {
-			t.Errorf("Expected query to start with ?, got %q", r.URL.RawQuery)
-		}
+		testFormValues(t, r, values{"owner[]": "octocat"})
 		fmt.Fprint(w, "[]")
 	})
 
@@ -357,13 +320,7 @@ func TestOrganizationsService_ListFineGrainedPersonalAccessTokenRequests_tokenID
 
 	mux.HandleFunc("/orgs/o/personal-access-token-requests", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		// When only TokenID is set (no Owner, ListOptions, etc.), addOptions adds no query params, so URL gets "?" + token_id[]=...
-		if !strings.Contains(r.URL.RawQuery, "token_id[]=") {
-			t.Errorf("Expected query to contain token_id[]=, got %q", r.URL.RawQuery)
-		}
-		if strings.HasPrefix(r.URL.RawQuery, "&") {
-			t.Errorf("Expected query not to start with & (token_id should be first param with ?), got %q", r.URL.RawQuery)
-		}
+		testFormValues(t, r, values{"token_id[]": "11579703"})
 		fmt.Fprint(w, "[]")
 	})
 

github/repos_contents_test.go

@@ -1010,10 +1010,7 @@ func TestRepositoriesService_GetContents_NoTrailingSlashInDirectoryApiPath(t *te
 
 	mux.HandleFunc("/repos/o/r/contents/.github", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		query := r.URL.Query()
-		if query.Get("ref") != "mybranch" {
-			t.Errorf("Repositories.GetContents returned %+v, want %+v", query.Get("ref"), "mybranch")
-		}
+		testFormValues(t, r, values{"ref": "mybranch"})
 		fmt.Fprint(w, `{}`)
 	})
 	ctx := t.Context()

github/repos_releases_test.go

@@ -989,10 +989,7 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_AbsoluteTemplate(t *t
 
 	mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "POST")
-		// Expect name query param created by addOptions after trimming template.
-		if got := r.URL.Query().Get("name"); got != "abs.txt" {
-			t.Errorf("Expected name query param 'abs.txt', got %q", got)
-		}
+		testFormValues(t, r, values{"name": "abs.txt"})
 		fmt.Fprint(w, `{"id":1}`)
 	})
 

github/security_advisories_test.go

@@ -551,11 +551,7 @@ func TestSecurityAdvisoriesService_ListRepositorySecurityAdvisoriesForOrg_NotFou
 
 	mux.HandleFunc("/orgs/o/security-advisories", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-
-		query := r.URL.Query()
-		if query.Get("state") != "draft" {
-			t.Errorf("ListRepositorySecurityAdvisoriesForOrg returned %+v, want %+v", query.Get("state"), "draft")
-		}
+		testFormValues(t, r, values{"state": "draft"})
 
 		http.NotFound(w, r)
 	})
@@ -682,11 +678,7 @@ func TestSecurityAdvisoriesService_ListRepositorySecurityAdvisories_NotFound(t *
 
 	mux.HandleFunc("/repos/o/r/security-advisories", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-
-		query := r.URL.Query()
-		if query.Get("state") != "draft" {
-			t.Errorf("ListRepositorySecurityAdvisories returned %+v, want %+v", query.Get("state"), "draft")
-		}
+		testFormValues(t, r, values{"state": "draft"})
 
 		http.NotFound(w, r)
 	})