Skip to content

Commit

Permalink
Fix invalid JSON struct tag, and other linter issues
Browse files Browse the repository at this point in the history
This change is a pretty noisy change in that I went through and cleaned up all
of the issues that my linters flagged. I wanted to get this done, so that we can
add them as automatic CI checks for future PRs.

In the process I did find one critical bug, where we had a malformed JSON struct
tag:

https://github.com/PagerDuty/go-pagerduty/blob/69ade4b95ef0ff1d582a7d9f226a98f9db886df1/webhook.go#L44

There are two linter issues we cannot fix until v1.5.0, because they are
breaking changes. As such, the v1.5.0 development line would be the first place
we can enforce those CI checks.

Fixes #317
  • Loading branch information
theckman committed Mar 30, 2021
1 parent 1b67a25 commit b64a98d
Show file tree
Hide file tree
Showing 34 changed files with 357 additions and 369 deletions.
4 changes: 2 additions & 2 deletions ability.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ func (c *Client) ListAbilitiesWithContext(ctx context.Context) (*ListAbilityResp
return &result, nil
}

// TestAbility Check if your account has the given ability.
// TestAbility checks if your account has the given ability.
func (c *Client) TestAbility(ability string) error {
return c.TestAbilityWithContext(context.Background(), ability)
}

// TestAbility Check if your account has the given ability.
// TestAbilityWithContext checks if your account has the given ability.
func (c *Client) TestAbilityWithContext(ctx context.Context, ability string) error {
_, err := c.get(ctx, "/abilities/"+ability)
return err
Expand Down
11 changes: 5 additions & 6 deletions ability_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ func TestAbility_ListAbilities(t *testing.T) {

mux.HandleFunc("/abilities", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
w.Write([]byte(`{"abilities": ["sso"]}`))
_, _ = w.Write([]byte(`{"abilities": ["sso"]}`))
})

var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
want := &ListAbilityResponse{Abilities: []string{"sso"}}

res, err := client.ListAbilities()

if err != nil {
t.Fatal(err)
}
Expand All @@ -35,7 +34,7 @@ func TestAbility_ListAbilitiesFailure(t *testing.T) {
w.WriteHeader(http.StatusForbidden)
})

var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}

if _, err := client.ListAbilities(); err == nil {
t.Fatal("expected error; got nil")
Expand All @@ -51,7 +50,7 @@ func TestAbility_TestAbility(t *testing.T) {
w.WriteHeader(http.StatusNoContent)
})

var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}

if err := client.TestAbility("sso"); err != nil {
t.Fatal(err)
Expand All @@ -67,7 +66,7 @@ func TestAbility_TestAbilityFailure(t *testing.T) {
w.WriteHeader(http.StatusForbidden)
})

var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}

if err := client.TestAbility("sso"); err == nil {
t.Fatal("expected error; got nil")
Expand Down
21 changes: 10 additions & 11 deletions addon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ func TestAddon_List(t *testing.T) {

mux.HandleFunc("/addons", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
w.Write([]byte(`{"addons": [{"name": "Internal Status Page"}]}`))
_, _ = w.Write([]byte(`{"addons": [{"name": "Internal Status Page"}]}`))
})
var listObj = APIListObject{Limit: 0, Offset: 0, More: false, Total: 0}
listObj := APIListObject{Limit: 0, Offset: 0, More: false, Total: 0}
var opts ListAddonOptions
var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}

res, err := client.ListAddons(opts)
want := &ListAddonResponse{
Expand Down Expand Up @@ -43,10 +43,10 @@ func TestAddon_Install(t *testing.T) {
mux.HandleFunc("/addons", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
w.WriteHeader(http.StatusCreated)
w.Write([]byte(`{"addon": {"name": "Internal Status Page", "id": "1"}}`))
_, _ = w.Write([]byte(`{"addon": {"name": "Internal Status Page", "id": "1"}}`))
})

var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}

res, err := client.InstallAddon(input)

Expand All @@ -68,9 +68,9 @@ func TestAddon_Get(t *testing.T) {

mux.HandleFunc("/addons/1", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
w.Write([]byte(`{"addon": {"id": "1"}}`))
_, _ = w.Write([]byte(`{"addon": {"id": "1"}}`))
})
var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}

res, err := client.GetAddon("1")

Expand All @@ -91,9 +91,9 @@ func TestAddon_Update(t *testing.T) {

mux.HandleFunc("/addons/1", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "PUT")
w.Write([]byte(`{"addon": {"name": "Internal Status Page", "id": "1"}}`))
_, _ = w.Write([]byte(`{"addon": {"name": "Internal Status Page", "id": "1"}}`))
})
var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}

input := Addon{
Name: "Internal Status Page",
Expand Down Expand Up @@ -121,9 +121,8 @@ func TestAddon_Delete(t *testing.T) {
testMethod(t, r, "DELETE")
w.WriteHeader(http.StatusNoContent)
})
var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
err := client.DeleteAddon("1")

if err != nil {
t.Fatal(err)
}
Expand Down
33 changes: 21 additions & 12 deletions analytics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,17 @@ func TestAnalytics_GetAggregatedIncidentData(t *testing.T) {
}

bytesAnalyticsResponse, err := json.Marshal(analyticsResponse)
testErrCheck(t, "json.Marshal()", "", err)

mux.HandleFunc("/analytics/metrics/incidents/all", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
w.Write(bytesAnalyticsResponse)
_, _ = w.Write(bytesAnalyticsResponse)
})

client := &Client{apiEndpoint: server.URL,
authToken: "foo",
HTTPClient: defaultHTTPClient,
client := &Client{
apiEndpoint: server.URL,
authToken: "foo",
HTTPClient: defaultHTTPClient,
}

res, err := client.GetAggregatedIncidentData(context.Background(), analyticsRequest)
Expand Down Expand Up @@ -75,14 +78,17 @@ func TestAnalytics_GetAggregatedServiceData(t *testing.T) {
TimeZone: "Etc/UTC",
}
bytesAnalyticsResponse, err := json.Marshal(analyticsResponse)
testErrCheck(t, "json.Marshal()", "", err)

mux.HandleFunc("/analytics/metrics/incidents/services", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
w.Write(bytesAnalyticsResponse)
_, _ = w.Write(bytesAnalyticsResponse)
})

client := &Client{apiEndpoint: server.URL,
authToken: "foo",
HTTPClient: defaultHTTPClient,
client := &Client{
apiEndpoint: server.URL,
authToken: "foo",
HTTPClient: defaultHTTPClient,
}

res, err := client.GetAggregatedServiceData(context.Background(), analyticsRequest)
Expand Down Expand Up @@ -120,14 +126,17 @@ func TestAnalytics_GetAggregatedTeamData(t *testing.T) {
TimeZone: "Etc/UTC",
}
bytesAnalyticsResponse, err := json.Marshal(analyticsResponse)
testErrCheck(t, "json.Marshal()", "", err)

mux.HandleFunc("/analytics/metrics/incidents/teams", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
w.Write(bytesAnalyticsResponse)
_, _ = w.Write(bytesAnalyticsResponse)
})

client := &Client{apiEndpoint: server.URL,
authToken: "foo",
HTTPClient: defaultHTTPClient,
client := &Client{
apiEndpoint: server.URL,
authToken: "foo",
HTTPClient: defaultHTTPClient,
}

res, err := client.GetAggregatedTeamData(context.Background(), analyticsRequest)
Expand Down
23 changes: 11 additions & 12 deletions business_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ func TestBusinessService_List(t *testing.T) {

mux.HandleFunc("/business_services/", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
w.Write([]byte(`{"business_services": [{"id": "1"}]}`))
_, _ = w.Write([]byte(`{"business_services": [{"id": "1"}]}`))
})

var listObj = APIListObject{Limit: 0, Offset: 0, More: false, Total: 0}
var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
var opts = ListBusinessServiceOptions{
listObj := APIListObject{Limit: 0, Offset: 0, More: false, Total: 0}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
opts := ListBusinessServiceOptions{
APIListObject: listObj,
}
res, err := client.ListBusinessServices(opts)
Expand All @@ -44,10 +44,10 @@ func TestBusinessService_Create(t *testing.T) {

mux.HandleFunc("/business_services", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
w.Write([]byte(`{"business_service": {"id": "1", "name": "foo"}}`))
_, _ = w.Write([]byte(`{"business_service": {"id": "1", "name": "foo"}}`))
})

var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
input := &BusinessService{
Name: "foo",
}
Expand All @@ -71,10 +71,10 @@ func TestBusinessService_Get(t *testing.T) {

mux.HandleFunc("/business_services/1", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
w.Write([]byte(`{"business_service": {"id": "1", "name":"foo"}}`))
_, _ = w.Write([]byte(`{"business_service": {"id": "1", "name":"foo"}}`))
})

var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
ruleSetID := "1"

res, _, err := client.GetBusinessService(ruleSetID)
Expand Down Expand Up @@ -109,10 +109,10 @@ func TestBusinessService_Update(t *testing.T) {
t.Fatalf("got ID in the body when we were not supposed to")
}

w.Write([]byte(`{"business_service": {"id": "1", "name":"foo"}}`))
_, _ = w.Write([]byte(`{"business_service": {"id": "1", "name":"foo"}}`))
})

var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
input := &BusinessService{
ID: "1",
Name: "foo",
Expand All @@ -139,10 +139,9 @@ func TestBusinessService_Delete(t *testing.T) {
testMethod(t, r, "DELETE")
})

var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient}
ID := "1"
err := client.DeleteBusinessService(ID)

if err != nil {
t.Fatal(err)
}
Expand Down
8 changes: 3 additions & 5 deletions change_events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ func TestChangeEvent_Create(t *testing.T) {
"/v2/change/enqueue", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
w.WriteHeader(http.StatusAccepted)
w.Write([]byte(`{"message": "Change event processed", "status": "success"}`))
_, _ = w.Write([]byte(`{"message": "Change event processed", "status": "success"}`))
},
)

var client = &Client{
client := &Client{
v2EventsAPIEndpoint: server.URL,
apiEndpoint: server.URL,
authToken: "foo",
Expand Down Expand Up @@ -60,7 +60,6 @@ func TestChangeEvent_Create(t *testing.T) {
}

res, err := client.CreateChangeEvent(ce)

if err != nil {
t.Fatal(err)
}
Expand All @@ -82,7 +81,7 @@ func TestChangeEvent_CreateWithPayloadVerification(t *testing.T) {
},
)

var client = &Client{
client := &Client{
v2EventsAPIEndpoint: server.URL,
apiEndpoint: server.URL,
authToken: "foo",
Expand Down Expand Up @@ -111,5 +110,4 @@ func TestChangeEvent_CreateWithPayloadVerification(t *testing.T) {
}

_, _ = client.CreateChangeEvent(ce)

}
5 changes: 4 additions & 1 deletion client.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ type APIReference struct {
Type string `json:"type,omitempty"`
}

// APIDetails are the fields required to represent a details non-hydrated
// object.
type APIDetails struct {
Type string `json:"type,omitempty"`
Details string `json:"details,omitempty"`
Expand Down Expand Up @@ -312,7 +314,8 @@ func (c *Client) do(ctx context.Context, method, path string, body io.Reader, he
}

func (c *Client) decodeJSON(resp *http.Response, payload interface{}) error {
defer resp.Body.Close()
defer func() { _ = resp.Body.Close() }() // explicitly discard error

decoder := json.NewDecoder(resp.Body)
return decoder.Decode(payload)
}
Expand Down
3 changes: 2 additions & 1 deletion constants.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package pagerduty

const (
Version = "1.1.2"
// Version is current version of this client.
Version = "1.4.0"
)

0 comments on commit b64a98d

Please sign in to comment.