From 21abff16f75d0bab0c0a418c4f81aa9e57fd6958 Mon Sep 17 00:00:00 2001 From: Geraldo Castro Date: Sun, 26 Feb 2023 23:38:29 -0300 Subject: [PATCH 1/7] Add the expected test case to mount the query string --- github/github_test.go | 13 +++++++++++++ github/scim_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/github/github_test.go b/github/github_test.go index 0c969404341..85581a91b37 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -169,6 +169,19 @@ func testJSONMarshal(t *testing.T, v interface{}, want string) { } } +func testAddOptions(t *testing.T, url string, v interface{}, want string) { + t.Helper() + + u, err := addOptions(url, v) + if err != nil { + t.Errorf("Unable to add %#v as query parameters", v) + } + + if u != want { + t.Errorf("addOptions(%q, %#v) returned %s, want %s", url, v, u, want) + } +} + // Test how bad options are handled. Method f under test should // return an error. func testBadOptions(t *testing.T, methodName string, f func() error) { diff --git a/github/scim_test.go b/github/scim_test.go index c2663bb776c..334ba339259 100644 --- a/github/scim_test.go +++ b/github/scim_test.go @@ -7,6 +7,7 @@ package github import ( "context" + "fmt" "net/http" "testing" "time" @@ -446,6 +447,33 @@ func TestListSCIMProvisionedIdentitiesOptions_Marshal(t *testing.T) { testJSONMarshal(t, u, want) } +func TestListSCIMProvisionedIdentitiesOptions_addOptions(t *testing.T) { + url := "some/path" + + testAddOptions(t, url, &ListSCIMProvisionedIdentitiesOptions{}, url) + + testAddOptions( + t, + url, + &ListSCIMProvisionedIdentitiesOptions{ + StartIndex: Int(1), + Count: Int(10), + }, + fmt.Sprintf("%s?count=10&startIndex=1", url), + ) + + testAddOptions( + t, + url, + &ListSCIMProvisionedIdentitiesOptions{ + StartIndex: Int(1), + Count: Int(10), + Filter: String("test"), + }, + fmt.Sprintf("%s?count=10&filter=test&startIndex=1", url), + ) +} + func TestSCIMUserName_Marshal(t *testing.T) { testJSONMarshal(t, &SCIMUserName{}, `{ "givenName":"","familyName":"" From 269b9735fcf9e09b6ef8563747a38fa9fd33b4f6 Mon Sep 17 00:00:00 2001 From: Geraldo Castro Date: Sun, 26 Feb 2023 23:39:26 -0300 Subject: [PATCH 2/7] Change 'json' tags to 'url' in ListSCIMProvisionedIdentitiesOptions struct --- github/scim.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/github/scim.go b/github/scim.go index 70f81ed14e6..7deee6be4b5 100644 --- a/github/scim.go +++ b/github/scim.go @@ -69,14 +69,14 @@ type SCIMProvisionedIdentities struct { // // Github API docs: https://docs.github.com/en/rest/scim#list-scim-provisioned-identities--parameters type ListSCIMProvisionedIdentitiesOptions struct { - StartIndex *int `json:"startIndex,omitempty"` // Used for pagination: the index of the first result to return. (Optional.) - Count *int `json:"count,omitempty"` // Used for pagination: the number of results to return. (Optional.) + StartIndex *int `url:"startIndex,omitempty"` // Used for pagination: the index of the first result to return. (Optional.) + Count *int `url:"count,omitempty"` // Used for pagination: the number of results to return. (Optional.) // Filter results using the equals query parameter operator (eq). // You can filter results that are equal to id, userName, emails, and external_id. // For example, to search for an identity with the userName Octocat, you would use this query: ?filter=userName%20eq%20\"Octocat\". // To filter results for the identity with the email octocat@github.com, you would use this query: ?filter=emails%20eq%20\"octocat@github.com\". // (Optional.) - Filter *string `json:"filter,omitempty"` + Filter *string `url:"filter,omitempty"` } // ListSCIMProvisionedIdentities lists SCIM provisioned identities. From f593c214114c5e1801476baf49bcdfdd0bb944cf Mon Sep 17 00:00:00 2001 From: Geraldo Castro Date: Sun, 5 Mar 2023 23:52:23 -0300 Subject: [PATCH 3/7] Remove unnecessary test case --- github/scim_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/github/scim_test.go b/github/scim_test.go index 334ba339259..8a9aea427e1 100644 --- a/github/scim_test.go +++ b/github/scim_test.go @@ -429,24 +429,6 @@ func TestUpdateAttributeForSCIMUserOptions_Marshal(t *testing.T) { testJSONMarshal(t, u, want) } -func TestListSCIMProvisionedIdentitiesOptions_Marshal(t *testing.T) { - testJSONMarshal(t, &ListSCIMProvisionedIdentitiesOptions{}, `{}`) - - u := &ListSCIMProvisionedIdentitiesOptions{ - StartIndex: Int(1), - Count: Int(10), - Filter: String("test"), - } - - want := `{ - "startIndex": 1, - "count": 10, - "filter": "test" - }` - - testJSONMarshal(t, u, want) -} - func TestListSCIMProvisionedIdentitiesOptions_addOptions(t *testing.T) { url := "some/path" From 03b32398cf57b8acb67296e8b01d41e5bf572c3c Mon Sep 17 00:00:00 2001 From: Geraldo Castro Date: Mon, 6 Mar 2023 19:29:00 -0300 Subject: [PATCH 4/7] Add a testJSONMarshal test case and improve the variable name --- github/github_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/github/github_test.go b/github/github_test.go index 85581a91b37..08b029bc393 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -172,13 +172,15 @@ func testJSONMarshal(t *testing.T, v interface{}, want string) { func testAddOptions(t *testing.T, url string, v interface{}, want string) { t.Helper() - u, err := addOptions(url, v) + testJSONMarshal(t, &ListSCIMProvisionedIdentitiesOptions{}, `{}`) + + got, err := addOptions(url, v) if err != nil { t.Errorf("Unable to add %#v as query parameters", v) } - if u != want { - t.Errorf("addOptions(%q, %#v) returned %s, want %s", url, v, u, want) + if got != want { + t.Errorf("addOptions(%q, %#v) returned %v, want %v", url, v, got, want) } } From 0d9134141e3cf98f08984b41216b4fe90e4ef870 Mon Sep 17 00:00:00 2001 From: Geraldo Castro Date: Mon, 6 Mar 2023 19:42:55 -0300 Subject: [PATCH 5/7] Move the test to the right place --- github/github_test.go | 2 -- github/scim_test.go | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/github/github_test.go b/github/github_test.go index 08b029bc393..9b14fd744df 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -172,8 +172,6 @@ func testJSONMarshal(t *testing.T, v interface{}, want string) { func testAddOptions(t *testing.T, url string, v interface{}, want string) { t.Helper() - testJSONMarshal(t, &ListSCIMProvisionedIdentitiesOptions{}, `{}`) - got, err := addOptions(url, v) if err != nil { t.Errorf("Unable to add %#v as query parameters", v) diff --git a/github/scim_test.go b/github/scim_test.go index 8a9aea427e1..dec922d7cb5 100644 --- a/github/scim_test.go +++ b/github/scim_test.go @@ -430,6 +430,8 @@ func TestUpdateAttributeForSCIMUserOptions_Marshal(t *testing.T) { } func TestListSCIMProvisionedIdentitiesOptions_addOptions(t *testing.T) { + testJSONMarshal(t, &ListSCIMProvisionedIdentitiesOptions{}, `{}`) + url := "some/path" testAddOptions(t, url, &ListSCIMProvisionedIdentitiesOptions{}, url) From bc5dd90aaac7a50efe6a4848cfe61040c6653742 Mon Sep 17 00:00:00 2001 From: Geraldo Castro Date: Tue, 7 Mar 2023 20:02:28 -0300 Subject: [PATCH 6/7] Add a step to verify if all fields in the struct have the url tag --- github/github_test.go | 16 +++++++++++++++- github/scim_test.go | 6 +++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/github/github_test.go b/github/github_test.go index 9b14fd744df..8681cc000fb 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -169,9 +169,23 @@ func testJSONMarshal(t *testing.T, v interface{}, want string) { } } -func testAddOptions(t *testing.T, url string, v interface{}, want string) { +// Test whether the v fields have the url tag and the parsing of v +// produces query parameters that corresponds to the want string. +func testAddURLOptions(t *testing.T, url string, v interface{}, want string) { t.Helper() + vt := reflect.Indirect(reflect.ValueOf(v)).Type() + for i := 0; i < vt.NumField(); i++ { + field := vt.Field(i) + if alias, ok := field.Tag.Lookup("url"); ok { + if alias == "" { + t.Errorf("The field %+v has a blank url tag", field) + } + } else { + t.Errorf("The field %+v has no url tag specified", field) + } + } + got, err := addOptions(url, v) if err != nil { t.Errorf("Unable to add %#v as query parameters", v) diff --git a/github/scim_test.go b/github/scim_test.go index dec922d7cb5..84c7ff3df99 100644 --- a/github/scim_test.go +++ b/github/scim_test.go @@ -434,9 +434,9 @@ func TestListSCIMProvisionedIdentitiesOptions_addOptions(t *testing.T) { url := "some/path" - testAddOptions(t, url, &ListSCIMProvisionedIdentitiesOptions{}, url) + testAddURLOptions(t, url, &ListSCIMProvisionedIdentitiesOptions{}, url) - testAddOptions( + testAddURLOptions( t, url, &ListSCIMProvisionedIdentitiesOptions{ @@ -446,7 +446,7 @@ func TestListSCIMProvisionedIdentitiesOptions_addOptions(t *testing.T) { fmt.Sprintf("%s?count=10&startIndex=1", url), ) - testAddOptions( + testAddURLOptions( t, url, &ListSCIMProvisionedIdentitiesOptions{ From f8b4ff566a9f3ffed82a9346df9aebfdeec02784 Mon Sep 17 00:00:00 2001 From: Geraldo Castro Date: Tue, 7 Mar 2023 20:07:37 -0300 Subject: [PATCH 7/7] Add/improve a string that better reflects the test case --- github/scim_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/github/scim_test.go b/github/scim_test.go index 84c7ff3df99..9e5274766aa 100644 --- a/github/scim_test.go +++ b/github/scim_test.go @@ -430,7 +430,11 @@ func TestUpdateAttributeForSCIMUserOptions_Marshal(t *testing.T) { } func TestListSCIMProvisionedIdentitiesOptions_addOptions(t *testing.T) { - testJSONMarshal(t, &ListSCIMProvisionedIdentitiesOptions{}, `{}`) + testJSONMarshal(t, &ListSCIMProvisionedIdentitiesOptions{}, `{ + "StartIndex": null, + "Count": null, + "Filter": null + }`) url := "some/path"