From 0feccadf8daf701f37cc421e0f139e2023e7cf80 Mon Sep 17 00:00:00 2001 From: Andy Grunwald Date: Sat, 26 Mar 2016 21:23:12 +0100 Subject: [PATCH] Fix #1: Add some unit tests --- errors.go | 10 +- errors_test.go | 82 +++++++++++++++++ jira_test.go | 241 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 3 deletions(-) create mode 100644 errors_test.go create mode 100644 jira_test.go diff --git a/errors.go b/errors.go index 5c5a316..66f2ee1 100644 --- a/errors.go +++ b/errors.go @@ -13,7 +13,11 @@ type ErrorResponse struct { } func (r *ErrorResponse) Error() string { - return fmt.Sprintf("%v %v: %d %v %+v", - r.Response.Request.Method, r.Response.Request.URL, - r.Response.StatusCode, r.ErrorMessages, r.Errors) + if r.Response == nil { + return fmt.Sprintf("%v %+v", r.ErrorMessages, r.Errors) + } else { + return fmt.Sprintf("%v %v: %d %v %+v", + r.Response.Request.Method, r.Response.Request.URL, + r.Response.StatusCode, r.ErrorMessages, r.Errors) + } } diff --git a/errors_test.go b/errors_test.go new file mode 100644 index 0000000..1ee46e0 --- /dev/null +++ b/errors_test.go @@ -0,0 +1,82 @@ +package jira + +import ( + "net/http" + "net/url" + "testing" +) + +func TestErrorResponse_Empty(t *testing.T) { + u, _ := url.Parse("https://issues.apache.org/jira/browse/MESOS-5040") + r := &http.Response{ + Request: &http.Request{ + Method: "POST", + URL: u, + }, + StatusCode: 200, + } + + mockData := []struct { + Response ErrorResponse + Expected string + }{ + { + Response: ErrorResponse{}, + Expected: "[] map[]", + }, + { + Response: ErrorResponse{ + ErrorMessages: []string{"foo", "bar"}, + }, + Expected: "[foo bar] map[]", + }, + { + Response: ErrorResponse{ + Errors: map[string]string{"Foo": "Bar"}, + }, + Expected: "[] map[Foo:Bar]", + }, + { + Response: ErrorResponse{ + ErrorMessages: []string{"foo", "bar"}, + Errors: map[string]string{"Foo": "Bar"}, + }, + Expected: "[foo bar] map[Foo:Bar]", + }, + { + Response: ErrorResponse{ + Response: r, + }, + Expected: "POST https://issues.apache.org/jira/browse/MESOS-5040: 200 [] map[]", + }, + { + Response: ErrorResponse{ + Response: r, + ErrorMessages: []string{"foo", "bar"}, + }, + Expected: "POST https://issues.apache.org/jira/browse/MESOS-5040: 200 [foo bar] map[]", + }, + { + Response: ErrorResponse{ + Response: r, + Errors: map[string]string{"Foo": "Bar"}, + }, + Expected: "POST https://issues.apache.org/jira/browse/MESOS-5040: 200 [] map[Foo:Bar]", + }, + { + Response: ErrorResponse{ + Response: r, + ErrorMessages: []string{"foo", "bar"}, + Errors: map[string]string{"Foo": "Bar"}, + }, + Expected: "POST https://issues.apache.org/jira/browse/MESOS-5040: 200 [foo bar] map[Foo:Bar]", + }, + } + + for _, data := range mockData { + got := data.Response.Error() + if got != data.Expected { + t.Errorf("Response is different as expected. Expected \"%s\". Got \"%s\"", data.Expected, got) + } + } +} diff --git a/jira_test.go b/jira_test.go new file mode 100644 index 0000000..83f9a89 --- /dev/null +++ b/jira_test.go @@ -0,0 +1,241 @@ +package jira + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "net/url" + "reflect" + "testing" + "time" +) + +const ( + testJIRAInstanceURL = "https://issues.apache.org/jira/" +) + +var ( + // testMux is the HTTP request multiplexer used with the test server. + testMux *http.ServeMux + + // testClient is the JIRA client being tested. + testClient *Client + + // testServer is a test HTTP server used to provide mock API responses. + testServer *httptest.Server +) + +type testValues map[string]string + +// setup sets up a test HTTP server along with a jira.Client that is configured to talk to that test server. +// Tests should register handlers on mux which provide mock responses for the API method being tested. +func setup() { + // Test server + testMux = http.NewServeMux() + testServer = httptest.NewServer(testMux) + + // jira client configured to use test server + testClient, _ = NewClient(nil, testServer.URL) +} + +// teardown closes the test HTTP server. +func teardown() { + testServer.Close() +} + +func TestNewClient_WrongUrl(t *testing.T) { + c, err := NewClient(nil, "://issues.apache.org/jira/") + + if err == nil { + t.Error("Expected an error. Got none") + } + if c != nil { + t.Errorf("Expected no client. Got %+v", c) + } +} + +func TestNewClient_WithHttpClient(t *testing.T) { + httpClient := http.DefaultClient + httpClient.Timeout = 10 * time.Minute + c, err := NewClient(httpClient, testJIRAInstanceURL) + + if err != nil { + t.Errorf("Got an error: %s", err) + } + if c == nil { + t.Error("Expected a client. Got none") + } + if !reflect.DeepEqual(c.client, httpClient) { + t.Errorf("HTTP clients are not equal. Injected %+v, got %+v", httpClient, c.client) + } +} + +func TestNewClient_WithServices(t *testing.T) { + c, err := NewClient(nil, testJIRAInstanceURL) + + if err != nil { + t.Errorf("Got an error: %s", err) + } + if c.Authentication == nil { + t.Error("No AuthenticationService provided") + } + if c.Issue == nil { + t.Error("No IssueService provided") + } +} + +func TestCheckResponse_GoodResults(t *testing.T) { + codes := []int{ + http.StatusOK, http.StatusPartialContent, 299, + } + + for _, c := range codes { + r := &http.Response{ + StatusCode: c, + } + if err := CheckResponse(r); err != nil { + t.Errorf("CheckResponse throws an error: %s", err) + } + } +} + +func TestNewRequest(t *testing.T) { + c, err := NewClient(nil, testJIRAInstanceURL) + if err != nil { + t.Errorf("An error occured. Expected nil. Got %+v.", err) + } + + inURL, outURL := "rest/api/2/issue/", testJIRAInstanceURL+"rest/api/2/issue/" + inBody, outBody := &Issue{Key: "MESOS"}, `{"key":"MESOS"}`+"\n" + req, _ := c.NewRequest("GET", inURL, inBody) + + // Test that relative URL was expanded + if got, want := req.URL.String(), outURL; got != want { + t.Errorf("NewRequest(%q) URL is %v, want %v", inURL, got, want) + } + + // Test that body was JSON encoded + body, _ := ioutil.ReadAll(req.Body) + if got, want := string(body), outBody; got != want { + t.Errorf("NewRequest(%q) Body is %v, want %v", inBody, got, want) + } +} + +func TestNewRequest_InvalidJSON(t *testing.T) { + c, err := NewClient(nil, testJIRAInstanceURL) + if err != nil { + t.Errorf("An error occured. Expected nil. Got %+v.", err) + } + + type T struct { + A map[int]interface{} + } + _, err = c.NewRequest("GET", "/", &T{}) + + if err == nil { + t.Error("Expected error to be returned.") + } + if err, ok := err.(*json.UnsupportedTypeError); !ok { + t.Errorf("Expected a JSON error; got %+v.", err) + } +} + +func testURLParseError(t *testing.T, err error) { + if err == nil { + t.Errorf("Expected error to be returned") + } + if err, ok := err.(*url.Error); !ok || err.Op != "parse" { + t.Errorf("Expected URL parse error, got %+v", err) + } +} + +func TestNewRequest_BadURL(t *testing.T) { + c, err := NewClient(nil, testJIRAInstanceURL) + if err != nil { + t.Errorf("An error occured. Expected nil. Got %+v.", err) + } + _, err = c.NewRequest("GET", ":", nil) + testURLParseError(t, err) +} + +// If a nil body is passed to gerrit.NewRequest, make sure that nil is also passed to http.NewRequest. +// In most cases, passing an io.Reader that returns no content is fine, +// since there is no difference between an HTTP request body that is an empty string versus one that is not set at all. +// However in certain cases, intermediate systems may treat these differently resulting in subtle errors. +func TestNewRequest_EmptyBody(t *testing.T) { + c, err := NewClient(nil, testJIRAInstanceURL) + if err != nil { + t.Errorf("An error occured. Expected nil. Got %+v.", err) + } + req, err := c.NewRequest("GET", "/", nil) + if err != nil { + t.Fatalf("NewRequest returned unexpected error: %v", err) + } + if req.Body != nil { + t.Fatalf("constructed request contains a non-nil Body") + } +} + +func TestDo(t *testing.T) { + setup() + defer teardown() + + type foo struct { + A string + } + + testMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if m := "GET"; m != r.Method { + t.Errorf("Request method = %v, want %v", r.Method, m) + } + fmt.Fprint(w, `{"A":"a"}`) + }) + + req, _ := testClient.NewRequest("GET", "/", nil) + body := new(foo) + testClient.Do(req, body) + + want := &foo{"a"} + if !reflect.DeepEqual(body, want) { + t.Errorf("Response body = %v, want %v", body, want) + } +} + +func TestDo_HTTPError(t *testing.T) { + setup() + defer teardown() + + testMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "Bad Request", 400) + }) + + req, _ := testClient.NewRequest("GET", "/", nil) + _, err := testClient.Do(req, nil) + + if err == nil { + t.Error("Expected HTTP 400 error.") + } +} + +// Test handling of an error caused by the internal http client's Do() function. +// A redirect loop is pretty unlikely to occur within the Gerrit API, but does allow us to exercise the right code path. +func TestDo_RedirectLoop(t *testing.T) { + setup() + defer teardown() + + testMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "/", http.StatusFound) + }) + + req, _ := testClient.NewRequest("GET", "/", nil) + _, err := testClient.Do(req, nil) + + if err == nil { + t.Error("Expected error to be returned.") + } + if err, ok := err.(*url.Error); !ok { + t.Errorf("Expected a URL error; got %+v.", err) + } +}