2769: code review comments

[arvados.git] / services / keep / src / keep / handler_test.go

diff --git a/services/keep/src/keep/handler_test.go b/services/keep/src/keep/handler_test.go
index 2998f60427298f019f21871352f7db26a380fedf..030e3338f6a7ade8ee4f23c2bc5221ca333588a7 100644 (file)
--- a/services/keep/src/keep/handler_test.go
+++ b/services/keep/src/keep/handler_test.go
@@ -44,7 +44,7 @@ func TestGetHandler(t *testing.T) {
 
        // Prepare two test Keep volumes. Our block is stored on the second volume.
        KeepVM = MakeTestVolumeManager(2)
-       defer func() { KeepVM.Quit() }()
+       defer KeepVM.Quit()
 
        vols := KeepVM.Volumes()
        if err := vols[0].Put(TEST_HASH, TEST_BLOCK); err != nil {
@@ -61,11 +61,11 @@ func TestGetHandler(t *testing.T) {
        permission_ttl = time.Duration(300) * time.Second
 
        var (
-               unsigned_locator  = "http://localhost:25107/" + TEST_HASH
+               unsigned_locator  = "/" + TEST_HASH
                valid_timestamp   = time.Now().Add(permission_ttl)
                expired_timestamp = time.Now().Add(-time.Hour)
-               signed_locator    = "http://localhost:25107/" + SignLocator(TEST_HASH, known_token, valid_timestamp)
-               expired_locator   = "http://localhost:25107/" + SignLocator(TEST_HASH, known_token, expired_timestamp)
+               signed_locator    = "/" + SignLocator(TEST_HASH, known_token, valid_timestamp)
+               expired_locator   = "/" + SignLocator(TEST_HASH, known_token, expired_timestamp)
        )
 
        // -----------------
@@ -153,7 +153,7 @@ func TestPutHandler(t *testing.T) {
 
        // Prepare two test Keep volumes.
        KeepVM = MakeTestVolumeManager(2)
-       defer func() { KeepVM.Quit() }()
+       defer KeepVM.Quit()
 
        // Set up a REST router for testing the handlers.
        rest := MakeRESTRouter()
@@ -163,7 +163,7 @@ func TestPutHandler(t *testing.T) {
 
        // Unauthenticated request, no server key
        // => OK (unsigned response)
-       unsigned_locator := "http://localhost:25107/" + TEST_HASH
+       unsigned_locator := "/" + TEST_HASH
        response := IssueRequest(rest,
                &RequestTester{
                        method:       "PUT",
@@ -247,7 +247,7 @@ func TestIndexHandler(t *testing.T) {
        // Include multiple blocks on different volumes, and
        // some metadata files (which should be omitted from index listings)
        KeepVM = MakeTestVolumeManager(2)
-       defer func() { KeepVM.Quit() }()
+       defer KeepVM.Quit()
 
        vols := KeepVM.Volumes()
        vols[0].Put(TEST_HASH, TEST_BLOCK)
@@ -262,30 +262,30 @@ func TestIndexHandler(t *testing.T) {
 
        unauthenticated_req := &RequestTester{
                method: "GET",
-               uri:    "http://localhost:25107/index",
+               uri:    "/index",
        }
        authenticated_req := &RequestTester{
                method:    "GET",
-               uri:       "http://localhost:25107/index",
+               uri:       "/index",
                api_token: known_token,
        }
        superuser_req := &RequestTester{
                method:    "GET",
-               uri:       "http://localhost:25107/index",
+               uri:       "/index",
                api_token: data_manager_token,
        }
        unauth_prefix_req := &RequestTester{
                method: "GET",
-               uri:    "http://localhost:25107/index/" + TEST_HASH[0:3],
+               uri:    "/index/" + TEST_HASH[0:3],
        }
        auth_prefix_req := &RequestTester{
                method:    "GET",
-               uri:       "http://localhost:25107/index/" + TEST_HASH[0:3],
+               uri:       "/index/" + TEST_HASH[0:3],
                api_token: known_token,
        }
        superuser_prefix_req := &RequestTester{
                method:    "GET",
-               uri:       "http://localhost:25107/index/" + TEST_HASH[0:3],
+               uri:       "/index/" + TEST_HASH[0:3],
                api_token: data_manager_token,
        }
 
@@ -415,7 +415,7 @@ func TestIndexHandler(t *testing.T) {
 
 // TestDeleteHandler
 //
-// Cases to test:
+// Cases tested:
 //
 //   With no token and with a non-data-manager token:
 //   * Delete existing block
@@ -446,7 +446,7 @@ func TestDeleteHandler(t *testing.T) {
        // Include multiple blocks on different volumes, and
        // some metadata files (which should be omitted from index listings)
        KeepVM = MakeTestVolumeManager(2)
-       defer func() { KeepVM.Quit() }()
+       defer KeepVM.Quit()
 
        vols := KeepVM.Volumes()
        vols[0].Put(TEST_HASH, TEST_BLOCK)
@@ -459,24 +459,24 @@ func TestDeleteHandler(t *testing.T) {
 
        unauth_req := &RequestTester{
                method: "DELETE",
-               uri:    "http://localhost:25107/" + TEST_HASH,
+               uri:    "/" + TEST_HASH,
        }
 
        user_req := &RequestTester{
                method:    "DELETE",
-               uri:       "http://localhost:25107/" + TEST_HASH,
+               uri:       "/" + TEST_HASH,
                api_token: user_token,
        }
 
        superuser_existing_block_req := &RequestTester{
                method:    "DELETE",
-               uri:       "http://localhost:25107/" + TEST_HASH,
+               uri:       "/" + TEST_HASH,
                api_token: data_manager_token,
        }
 
        superuser_nonexistent_block_req := &RequestTester{
                method:    "DELETE",
-               uri:       "http://localhost:25107/" + TEST_HASH_2,
+               uri:       "/" + TEST_HASH_2,
                api_token: data_manager_token,
        }
 
@@ -500,17 +500,19 @@ func TestDeleteHandler(t *testing.T) {
                Deleted int `json:"copies_deleted"`
                Failed  int `json:"copies_failed"`
        }
-       var dc deletecounter
+       var dc, expected_dc deletecounter
 
        response = IssueRequest(rest, superuser_nonexistent_block_req)
        ExpectStatusCode(t,
                "data manager request, nonexistent block",
-               http.StatusOK,
+               http.StatusNotFound,
                response)
        // Expect response {"copies_deleted":0,"copies_failed":0}
+       expected_dc = deletecounter{0, 0}
        json.NewDecoder(response.Body).Decode(&dc)
-       if dc.Deleted != 0 || dc.Failed != 0 {
-               t.Errorf("superuser_nonexistent_block_req: response = %+v", dc)
+       if dc != expected_dc {
+               t.Errorf("superuser_nonexistent_block_req\nexpected: %+v\nreceived: %+v",
+                       expected_dc, dc)
        }
 
        // Authenticated admin request for existing block.
@@ -520,9 +522,11 @@ func TestDeleteHandler(t *testing.T) {
                http.StatusOK,
                response)
        // Expect response {"copies_deleted":1,"copies_failed":0}
+       expected_dc = deletecounter{1, 0}
        json.NewDecoder(response.Body).Decode(&dc)
-       if dc.Deleted != 1 || dc.Failed != 0 {
-               t.Errorf("superuser_existing_block_req: response = %+v", dc)
+       if dc != expected_dc {
+               t.Errorf("superuser_existing_block_req\nexpected: %+v\nreceived: %+v",
+                       expected_dc, dc)
        }
        // Confirm the block has been deleted
        _, err := vols[0].Get(TEST_HASH)