Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add title to ManageIncidentOptions #372

Merged
merged 1 commit into from
Nov 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions incident.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ type ManageIncidentsOptions struct {
ID string `json:"id"`
Type string `json:"type"`
Status string `json:"status,omitempty"`
Title string `json:"title,omitempty"`
Priority *APIReference `json:"priority,omitempty"`
Assignments []Assignee `json:"assignments,omitempty"`
EscalationPolicy *APIReference `json:"escalation_policy,omitempty"`
Expand Down
56 changes: 56 additions & 0 deletions incident_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package pagerduty

import (
"encoding/json"
"io/ioutil"
"net/http"
"testing"
)
Expand Down Expand Up @@ -155,6 +157,60 @@ func TestIncident_Manage_priority(t *testing.T) {
testEqual(t, want, res)
}

func TestIncident_Manage_title(t *testing.T) {
setup()
defer teardown()

mux.HandleFunc("/incidents", func(w http.ResponseWriter, r *http.Request) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to assert that Title is serialized in the JSON and sent in the body. Can you update this handler to return an http.StatusBadRequest if the title JSON key is not the expected value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning http.StatusBadRequest result in unclear errors HTTP response with status code 422 does not contain Content-Type: application/json

Having clear errors will require to use an helper function in order to set proper headers and serialize the error in the right format: {"error":{"message": "my error", "code": 2100}}. This ends-up in mocking the api errors and testing the error handling code path which may complicate test maintenance in the future.

Looking at the code I found https://github.com/PagerDuty/go-pagerduty/blob/master/business_service_test.go#L100
This look like how http method test are handled https://github.com/PagerDuty/go-pagerduty/blob/master/client_test.go#L55

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I get that. I've a desire to rework the tests to more clearly assert that we're matching the API's expectations, so I was trying to avoid having more places where I need to worry about testing for that in the future. To your point, there are no patterns of how to really do that well yet.

testMethod(t, r, "PUT")

// test if title is present and correct
body, err := ioutil.ReadAll(r.Body)
if err != nil {
t.Fatal(err)
}
var data map[string][]ManageIncidentsOptions
if err := json.Unmarshal(body, &data); err != nil {
t.Fatal(err)
}
if len(data["incidents"]) == 0 {
t.Fatalf("no incidents, expect 1")
}
if data["incidents"][0].Title != "bar" {
t.Fatalf("expected incidents[0].title to be \"bar\" got \"%s\"", data["incidents"][0].Title)
}
_, _ = w.Write([]byte(`{"incidents": [{"title": "bar", "id": "1"}]}`))
})
listObj := APIListObject{Limit: 0, Offset: 0, More: false, Total: 0}
client := defaultTestClient(server.URL, "foo")
from := "foo@bar.com"

input := []ManageIncidentsOptions{
{
ID: "1",
Type: "incident",
Title: "bar",
},
}

want := &ListIncidentsResponse{
APIListObject: listObj,
Incidents: []Incident{
{
APIObject: APIObject{
ID: "1",
},
Title: "bar",
},
},
}
res, err := client.ManageIncidents(from, input)
if err != nil {
t.Fatal(err)
}
testEqual(t, want, res)
}

func TestIncident_Manage_assignments(t *testing.T) {
setup()
defer teardown()
Expand Down