Merge pull request #8514 from hashicorp/dnephin/testing-improvements-1

testing: small improvements to TestSessionCreate and testutil.retry

agent/http_test.go

@@ -1296,10 +1296,8 @@ func TestAllowedNets(t *testing.T) {
 
 // assertIndex tests that X-Consul-Index is set and non-zero
 func assertIndex(t *testing.T, resp *httptest.ResponseRecorder) {
-	header := resp.Header().Get("X-Consul-Index")
+	t.Helper()
-	if header == "" || header == "0" {
+	require.NoError(t, checkIndex(resp))
-		t.Fatalf("Bad: %v", header)
-	}
 }
 
 // checkIndex is like assertIndex but returns an error

agent/session_endpoint_test.go

@@ -97,7 +97,7 @@ func TestSessionCreate(t *testing.T) {
 			"Checks":    []types.CheckID{"consul"},
 			"LockDelay": "20s",
 		}
-		enc.Encode(raw)
+		require.NoError(r, enc.Encode(raw))
 
 		req, _ := http.NewRequest("PUT", "/v1/session/create", body)
 		resp := httptest.NewRecorder()
@@ -158,7 +158,7 @@ func TestSessionCreate_NodeChecks(t *testing.T) {
 			"NodeChecks": []types.CheckID{structs.SerfCheckID},
 			"LockDelay":  "20s",
 		}
-		enc.Encode(raw)
+		require.NoError(r, enc.Encode(raw))
 
 		req, _ := http.NewRequest("PUT", "/v1/session/create", body)
 		resp := httptest.NewRecorder()
@@ -216,7 +216,7 @@ func TestSessionCreate_Delete(t *testing.T) {
 			"LockDelay":  "20s",
 			"Behavior":   structs.SessionKeysDelete,
 		}
-		enc.Encode(raw)
+		require.NoError(r, enc.Encode(raw))
 
 		req, _ := http.NewRequest("PUT", "/v1/session/create", body)
 		resp := httptest.NewRecorder()
@@ -244,23 +244,21 @@ func TestSessionCreate_DefaultCheck(t *testing.T) {
 	defer a.Shutdown()
 	testrpc.WaitForTestAgent(t, a.RPC, "dc1")
 
-	// Associate session with node and 2 health checks
-	body := bytes.NewBuffer(nil)
-	enc := json.NewEncoder(body)
 	raw := map[string]interface{}{
 		"Name":      "my-cool-session",
 		"Node":      a.Config.NodeName,
 		"LockDelay": "20s",
 	}
-	enc.Encode(raw)
 
+	retry.Run(t, func(r *retry.R) {
+		body := bytes.NewBuffer(nil)
+		enc := json.NewEncoder(body)
+		require.NoError(r, enc.Encode(raw))
 		req, _ := http.NewRequest("PUT", "/v1/session/create", body)
 		resp := httptest.NewRecorder()
-	retry.Run(t, func(r *retry.R) {
 		obj, err := a.srv.SessionCreate(resp, req)
-		if err != nil {
+		require.NoError(r, err)
-			r.Fatalf("err: %v", err)
+		require.Equal(r, resp.Code, http.StatusOK)
-		}
 
 		want := structs.Session{
 			ID:         obj.(sessionCreateResponse).ID,
@@ -281,26 +279,23 @@ func TestSessionCreate_NoCheck(t *testing.T) {
 
 	testrpc.WaitForTestAgent(t, a.RPC, "dc1")
 
-	t.Run("no check fields should yield default serfHealth", func(t *testing.T) {
-		body := bytes.NewBuffer(nil)
-		enc := json.NewEncoder(body)
 	raw := map[string]interface{}{
 		"Name":      "my-cool-session",
 		"Node":      a.Config.NodeName,
 		"LockDelay": "20s",
 	}
-		enc.Encode(raw)
+
+	t.Run("no check fields should yield default serfHealth", func(t *testing.T) {
+		retry.Run(t, func(r *retry.R) {
+			body := bytes.NewBuffer(nil)
+			enc := json.NewEncoder(body)
+			require.NoError(r, enc.Encode(raw))
 
 			req, _ := http.NewRequest("PUT", "/v1/session/create", body)
 			resp := httptest.NewRecorder()
-		retry.Run(t, func(r *retry.R) {
 			obj, err := a.srv.SessionCreate(resp, req)
-			if err != nil {
+			require.NoError(r, err)
-				r.Fatalf("err: %v", err)
+			require.Equal(r, resp.Code, http.StatusOK, resp.Body.String())
-			}
-			if obj == nil {
-				r.Fatalf("expected a session")
-			}
 
 			want := structs.Session{
 				ID:         obj.(sessionCreateResponse).ID,
@@ -315,23 +310,22 @@ func TestSessionCreate_NoCheck(t *testing.T) {
 	})
 
 	t.Run("overwrite nodechecks to associate with no checks", func(t *testing.T) {
-		body := bytes.NewBuffer(nil)
-		enc := json.NewEncoder(body)
 		raw := map[string]interface{}{
 			"Name":       "my-cool-session",
 			"Node":       a.Config.NodeName,
 			"NodeChecks": []string{},
 			"LockDelay":  "20s",
 		}
-		enc.Encode(raw)
 
+		retry.Run(t, func(r *retry.R) {
+			body := bytes.NewBuffer(nil)
+			enc := json.NewEncoder(body)
+			require.NoError(r, enc.Encode(raw))
 			req, _ := http.NewRequest("PUT", "/v1/session/create", body)
 			resp := httptest.NewRecorder()
-		retry.Run(t, func(r *retry.R) {
 			obj, err := a.srv.SessionCreate(resp, req)
-			if err != nil {
+			require.NoError(r, err)
-				r.Fatalf("err: %v", err)
+			require.Equal(r, resp.Code, http.StatusOK)
-			}
 
 			want := structs.Session{
 				ID:         obj.(sessionCreateResponse).ID,
@@ -346,23 +340,23 @@ func TestSessionCreate_NoCheck(t *testing.T) {
 	})
 
 	t.Run("overwrite checks to associate with no checks", func(t *testing.T) {
-		body := bytes.NewBuffer(nil)
-		enc := json.NewEncoder(body)
 		raw := map[string]interface{}{
 			"Name":      "my-cool-session",
 			"Node":      a.Config.NodeName,
 			"Checks":    []string{},
 			"LockDelay": "20s",
 		}
-		enc.Encode(raw)
+
+		retry.Run(t, func(r *retry.R) {
+			body := bytes.NewBuffer(nil)
+			enc := json.NewEncoder(body)
+			require.NoError(r, enc.Encode(raw))
 
 			req, _ := http.NewRequest("PUT", "/v1/session/create", body)
 			resp := httptest.NewRecorder()
-		retry.Run(t, func(r *retry.R) {
 			obj, err := a.srv.SessionCreate(resp, req)
-			if err != nil {
+			require.NoError(r, err)
-				r.Fatalf("err: %v", err)
+			require.Equal(r, resp.Code, http.StatusOK)
-			}
 
 			want := structs.Session{
 				ID:         obj.(sessionCreateResponse).ID,
@@ -379,6 +373,7 @@ func TestSessionCreate_NoCheck(t *testing.T) {
 }
 
 func makeTestSession(t *testing.T, srv *HTTPServer) string {
+	t.Helper()
 	url := "/v1/session/create"
 	req, _ := http.NewRequest("PUT", url, nil)
 	resp := httptest.NewRecorder()
@@ -391,13 +386,14 @@ func makeTestSession(t *testing.T, srv *HTTPServer) string {
 }
 
 func makeTestSessionDelete(t *testing.T, srv *HTTPServer) string {
+	t.Helper()
 	// Create Session with delete behavior
 	body := bytes.NewBuffer(nil)
 	enc := json.NewEncoder(body)
 	raw := map[string]interface{}{
 		"Behavior": "delete",
 	}
-	enc.Encode(raw)
+	require.NoError(t, enc.Encode(raw))
 
 	url := "/v1/session/create"
 	req, _ := http.NewRequest("PUT", url, body)
@@ -411,13 +407,14 @@ func makeTestSessionDelete(t *testing.T, srv *HTTPServer) string {
 }
 
 func makeTestSessionTTL(t *testing.T, srv *HTTPServer, ttl string) string {
+	t.Helper()
 	// Create Session with TTL
 	body := bytes.NewBuffer(nil)
 	enc := json.NewEncoder(body)
 	raw := map[string]interface{}{
 		"TTL": ttl,
 	}
-	enc.Encode(raw)
+	require.NoError(t, enc.Encode(raw))
 
 	url := "/v1/session/create"
 	req, _ := http.NewRequest("PUT", url, body)
@@ -586,9 +583,9 @@ func TestSessionGet(t *testing.T) {
 		defer a.Shutdown()
 		testrpc.WaitForTestAgent(t, a.RPC, "dc1")
 
+		retry.Run(t, func(r *retry.R) {
 			req, _ := http.NewRequest("GET", "/v1/session/info/adf4238a-882b-9ddc-4a9d-5b6758e4159e", nil)
 			resp := httptest.NewRecorder()
-		retry.Run(t, func(r *retry.R) {
 			obj, err := a.srv.SessionGet(resp, req)
 			if err != nil {
 				r.Fatalf("err: %v", err)

agent/ui_endpoint_test.go

@@ -153,10 +153,8 @@ func TestUiNodeInfo(t *testing.T) {
 	req, _ := http.NewRequest("GET", fmt.Sprintf("/v1/internal/ui/node/%s", a.Config.NodeName), nil)
 	resp := httptest.NewRecorder()
 	obj, err := a.srv.UINodeInfo(resp, req)
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatalf("err: %v", err)
+	require.Equal(t, resp.Code, http.StatusOK)
-	}
-
 	assertIndex(t, resp)
 
 	// Should be 1 node for the server

sdk/testutil/retry/retry.go

@@ -23,6 +23,8 @@ import (
 
 // Failer is an interface compatible with testing.T.
 type Failer interface {
+	Helper()
+
 	// Log is called for the final test output
 	Log(args ...interface{})
 
@@ -116,6 +118,7 @@ func dedup(a []string) string {
 func run(r Retryer, t Failer, f func(r *R)) {
 	rr := &R{}
 	fail := func() {
+		t.Helper()
 		out := dedup(rr.output)
 		if out != "" {
 			t.Log(out)