diff --git a/github/github_test.go b/github/github_test.go index 034384fe47b..81b46a21b84 100644 --- a/github/github_test.go +++ b/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) } } diff --git a/github/orgs_audit_log_test.go b/github/orgs_audit_log_test.go index b6b518d15e1..c901ffc6860 100644 --- a/github/orgs_audit_log_test.go +++ b/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) diff --git a/github/orgs_personal_access_tokens_test.go b/github/orgs_personal_access_tokens_test.go index 568b8706ca3..3ad495f71d6 100644 --- a/github/orgs_personal_access_tokens_test.go +++ b/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, "[]") }) diff --git a/github/repos_contents_test.go b/github/repos_contents_test.go index d241f7a49f8..113aad6ebc4 100644 --- a/github/repos_contents_test.go +++ b/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() diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go index d81ecb7383e..92dc045ebb1 100644 --- a/github/repos_releases_test.go +++ b/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}`) }) diff --git a/github/security_advisories_test.go b/github/security_advisories_test.go index 9967686f12a..5e4e187f979 100644 --- a/github/security_advisories_test.go +++ b/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) })