Normalize internal/client/*_test.go

* Adjust test style for easy reading, by using literals instead of vars
* Move comparison to the `bytes` domain, instead of `string`
* Simplify testing code by using bytes.Buffer directly
* More consistent naming of variables and dummies (HANDLE_{FOO,BAR,BAD})
* Consistent testing style of gock.IsDone()
* Stick to 80-column

internal/client/get.go

@@ -7,7 +7,7 @@ import (
 )
 
 // GetData will perform the request and write the results on the provided writer
-func GetData(host, id, path string, wr io.Writer) error {
+func GetData(host, id, path string, w io.Writer) error {
 	url := host + "/handlers/" + id + path
-	return http.Get(url, "", nil, wr)
+	return http.Get(url, "", nil, w)
 }

internal/client/get_test.go

@@ -1,7 +1,6 @@
 package client
 
 import (
-	"bufio"
 	"bytes"
 	"net/http"
 	"testing"
@@ -12,13 +11,13 @@ import (
 func TestWriteContentToWriter(t *testing.T) {
 	defer gock.Off()
 	gock.New("http://localhost").
-		Get("/handlers/THIS-IS-THE-HANDLER-ID/request/body").
+		Get("/handlers/HANDLER_BAR/request/body").
 		Reply(http.StatusOK).
-		Body(bytes.NewReader([]byte("FOO")))
+		BodyString("FOO")
 
 	var b bytes.Buffer
-	buf := bufio.NewWriter(&b)
+	err := GetData(
-	err := GetData("http://localhost", "THIS-IS-THE-HANDLER-ID", "/request/body", buf)
+		"http://localhost", "HANDLER_BAR", "/request/body", &b)
 
 	if err != nil {
 		t.Errorf("Unexpected error: %q", err)
@@ -28,7 +27,7 @@ func TestWriteContentToWriter(t *testing.T) {
 		t.Errorf("Received content mismatch: %q != %q", b.Bytes(), []byte("FOO"))
 	}
 
-	if gock.IsDone() == false {
+	if !gock.IsDone() {
 		t.Error("No expected endpoint called")
 	}
 }
@@ -36,16 +35,17 @@ func TestWriteContentToWriter(t *testing.T) {
 func TestPropagateHTTPError(t *testing.T) {
 	defer gock.Off()
 	gock.New("http://localhost").
-		Get("/handlers/THIS-IS-THE-HANDLER-ID/request/body").
+		Get("/handlers/HANDLER_BAR/request/body").
 		Reply(http.StatusTeapot)
 
-	err := GetData("http://localhost", "THIS-IS-THE-HANDLER-ID", "/request/body", nil)
+	err := GetData(
+		"http://localhost", "HANDLER_BAR", "/request/body", nil)
 
 	if err == nil {
 		t.Errorf("Expected error not returned")
 	}
 
-	if gock.IsDone() == false {
+	if !gock.IsDone() {
 		t.Error("No expected endpoint called")
 	}
 }

internal/client/route_add_test.go

@@ -21,15 +21,19 @@ func TestSuccessOnCorrectRoute(t *testing.T) {
 		Reply(http.StatusCreated).
 		JSON(map[string]string{})
 
-	// TODO: As per the spec¹, the call should return a JSON body with the info of the new route
+	// TODO: As per the spec¹, the call should return a JSON body with the info
-	// We should consider this in the mocked server and in the test
+	// of the newly created route.  Should we consider this in the mocked server
+	// and in the test?
 	// ¹: https://github.com/BBVA/kapow/tree/master/spec#insert-a-route
-	err := AddRoute("http://localhost", "/hello", "GET", "", "echo Hello World | kapow set /response/body", nil)
+
+	err := AddRoute(
+		"http://localhost",
+		"/hello", "GET", "", "echo Hello World | kapow set /response/body", nil)
 	if err != nil {
 		t.Errorf("Unexpected error: %s", err)
 	}
 
-	if gock.IsDone() == false {
+	if !gock.IsDone() {
 		t.Error("Expected endpoint call not made")
 	}
 }

internal/client/route_list_test.go

@@ -2,28 +2,21 @@ package client
 
 import (
 	"bytes"
-	"fmt"
 	"net/http"
 	"testing"
 
 	gock "gopkg.in/h2non/gock.v1"
 )
 
-const (
+func TestListRoutesOKEmpty(t *testing.T) {
-	host = "http://localhost:8080"
-)
-
-func TestListRoutesEmpty(t *testing.T) {
-	descr := fmt.Sprintf("ListRoutes(%q, nil)", host)
-
 	defer gock.Off()
-	gock.New(host).
+	gock.New("http://localhost:8080").
 		Get("/routes").
 		Reply(http.StatusOK)
 
-	err := ListRoutes(host, nil)
+	err := ListRoutes("http://localhost:8080", nil)
 	if err != nil {
-		t.Errorf("%s: unexpected error %q", descr, err)
+		t.Errorf("Unexpected error %q", err)
 	}
 
 	if !gock.IsDone() {
@@ -31,23 +24,24 @@ func TestListRoutesEmpty(t *testing.T) {
 	}
 }
 
-func TestListRoutesSome(t *testing.T) {
+func TestListRoutesOKSome(t *testing.T) {
-	descr := fmt.Sprintf("ListRoutes(%q, buf)", host)
-
-	const want = "JSON array of some routes..."
-
 	defer gock.Off()
-	gock.New(host).
+	gock.New("http://localhost:8080").
 		Get("/routes").
 		Reply(http.StatusOK).
-		BodyString(want)
+		JSON([]map[string]string{
+			{"foo": "bar"},
+			{"bar": "foo"},
+		})
 
-	buf := new(bytes.Buffer)
+	var b bytes.Buffer
-	err := ListRoutes(host, buf)
+	err := ListRoutes("http://localhost:8080", &b)
 	if err != nil {
-		t.Errorf("%s: unexpected error: %q", descr, err)
+		t.Errorf("Unexpected error: %q", err)
-	} else if got := buf.String(); got != want {
+	} else if !bytes.Equal(
-		t.Errorf("%s: got %q, expected %q", descr, buf, want)
+		b.Bytes(), []byte(`[{"foo":"bar"},{"bar":"foo"}]`+"\n")) {
+		t.Errorf("Unexpected error: got %q, want %q",
+			b.String(), `[{"foo":"bar"},{"bar":"foo"}]`+"\n")
 	}
 
 	if !gock.IsDone() {

internal/client/route_remove_test.go

@@ -7,18 +7,15 @@ import (
 	gock "gopkg.in/h2non/gock.v1"
 )
 
-func TestRemoveRouteExistent(t *testing.T) {
+func TestRemoveRouteOKExistent(t *testing.T) {
-	const (
-		host    = "http://localhost:8080"
-		routeID = "ROUTE_ID_OLDER_BUT_IT_CHECKS_OUT"
-	)
-
 	defer gock.Off()
-	gock.New(host).Delete("/routes/" + routeID).Reply(http.StatusNoContent)
+	gock.New("http://localhost:8080").
+		Delete("/routes/ROUTE_FOO").
+		Reply(http.StatusNoContent)
 
-	err := RemoveRoute(host, routeID)
+	err := RemoveRoute("http://localhost:8080", "ROUTE_FOO")
 	if err != nil {
-		t.Errorf("unexpected error: ‘%s’", err)
+		t.Errorf("unexpected error: %q", err)
 	}
 
 	if !gock.IsDone() {
@@ -26,21 +23,17 @@ func TestRemoveRouteExistent(t *testing.T) {
 	}
 }
 
-func TestRemoveRouteNonExistent(t *testing.T) {
+func TestRemoveRouteErrorNonExistent(t *testing.T) {
-	const (
-		host    = "http://localhost:8080"
-		routeID = "ROUTE_THIS_ONE_WONT_WORK_BUDDY"
-	)
-	expected := http.StatusText(http.StatusNotFound)
-
 	defer gock.Off()
-	gock.New(host).Delete("/routes/" + routeID).Reply(http.StatusNotFound)
+	gock.New("http://localhost:8080").
+		Delete("/routes/ROUTE_BAD").
+		Reply(http.StatusNotFound)
 
-	err := RemoveRoute(host, routeID)
+	err := RemoveRoute("http://localhost:8080", "ROUTE_BAD")
 	if err == nil {
-		t.Errorf("error not reported for nonexistent route")
+		t.Errorf("Error not reported for nonexistent route")
-	} else if err.Error() != expected {
+	} else if err.Error() != "Not Found" {
-		t.Errorf("error mismatch: expected ‘%s’, got ‘%s’", expected, err)
+		t.Errorf(`Error mismatch: got %q, want "Not Found"`, err)
 	}
 
 	if !gock.IsDone() {

internal/client/set_test.go

@@ -10,41 +10,16 @@ import (
 	"github.com/BBVA/kapow/internal/client"
 )
 
-// Test that not found errors are detected as invalid handler id
+// Test an HTTP OK request
-func TestNotFound(t *testing.T) {
+func TestSetDataSuccessOnCorrectRequest(t *testing.T) {
-	expectedErr := "Not Found"
-	host := "http://localhost:8080"
-	hid := "inventedID"
-	path := "/response/status/code"
-	reader := strings.NewReader("200")
-
 	defer gock.Off()
+	gock.New("http://localhost:8080").
+		Put("/HANDLER_FOO/response/status/code").
+		Reply(http.StatusOK)
 
-	gock.New(host).Put("/" + hid + path).Reply(http.StatusNotFound)
+	if err := client.SetData(
-
+		"http://localhost:8080",
-	if err := client.SetData(host, hid, path, reader); err == nil {
+		"HANDLER_FOO", "/response/status/code", strings.NewReader("200")); err != nil {
-		t.Error("Expected error not present")
-	} else if err.Error() != expectedErr {
-		t.Errorf("Error don't match: expected \"%s\", got \"%s\"", expectedErr, err.Error())
-	}
-
-	if !gock.IsDone() {
-		t.Errorf("No endpoint called")
-	}
-}
-
-// Test a http ok request
-func TestOkRequest(t *testing.T) {
-	host := "http://localhost:8080"
-	hid := "HANDLER_XXXXXXXXXXXX"
-	path := "/response/status/code"
-	reader := strings.NewReader("200")
-
-	defer gock.Off()
-
-	gock.New(host).Put("/" + hid + path).Reply(http.StatusOK)
-
-	if err := client.SetData(host, hid, path, reader); err != nil {
 		t.Error("Unexpected error")
 	}
 
@@ -52,3 +27,23 @@ func TestOkRequest(t *testing.T) {
 		t.Errorf("No endpoint called")
 	}
 }
+
+// Test that Not Found errors are detected when an invalid handler id is sent
+func TestSetDataErrIfBadHandlerID(t *testing.T) {
+	defer gock.Off()
+	gock.New("http://localhost:8080").
+		Put("/HANDLER_BAD/response/status/code").
+		Reply(http.StatusNotFound)
+
+	if err := client.SetData(
+		"http://localhost:8080",
+		"HANDLER_BAD", "/response/status/code", strings.NewReader("200")); err == nil {
+		t.Error("Expected error not present")
+	} else if err.Error() != "Not Found" {
+		t.Errorf(`Error mismatch: expected "Not Found", got %q`, err)
+	}
+
+	if !gock.IsDone() {
+		t.Errorf("No endpoint called")
+	}
+}