From a3a323fffa3ed6758498d7216ea2c12923a1af0b Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" <4903+rboyer@users.noreply.github.com> Date: Wed, 7 Jun 2023 13:53:27 -0500 Subject: [PATCH] fix some testing.T retry.R mixups (#17600) Fix some linter warnings before updating the lint-consul-retry code in hashicorp/lint-consul-retry#4 backport of m #17600 onto release/1.15.x --- agent/agent_endpoint_test.go | 56 +++++++++---------- agent/consul/acl_endpoint_test.go | 4 +- agent/consul/client_test.go | 2 +- .../consul/multilimiter/multilimiter_test.go | 2 +- agent/consul/prepared_query_endpoint_test.go | 16 ++---- agent/dns_test.go | 6 +- .../services/peerstream/stream_test.go | 18 +++--- agent/local/state_test.go | 20 +++---- 8 files changed, 60 insertions(+), 64 deletions(-) diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 2c5ee9f3727a..3ab9fbb46e1d 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -1816,7 +1816,7 @@ func TestAgent_ReloadDoesNotTriggerWatch(t *testing.T) { for i := 1; i < 7; i++ { contents, err := os.ReadFile(tmpFile) if err != nil { - t.Fatalf("should be able to read file, but had: %#v", err) + r.Fatalf("should be able to read file, but had: %#v", err) } contentsStr = string(contents) if contentsStr != "" { @@ -1903,14 +1903,14 @@ func TestAgent_ReloadDoesNotTriggerWatch(t *testing.T) { ensureNothingCritical(r, "red-is-dead") if err := a.reloadConfigInternal(cfg2); err != nil { - t.Fatalf("got error %v want nil", err) + r.Fatalf("got error %v want nil", err) } // We check that reload does not go to critical ensureNothingCritical(r, "red-is-dead") ensureNothingCritical(r, "testing-agent-reload-001") - require.NoError(t, a.updateTTLCheck(checkID, api.HealthPassing, "testing-agent-reload-002")) + require.NoError(r, a.updateTTLCheck(checkID, api.HealthPassing, "testing-agent-reload-002")) ensureNothingCritical(r, "red-is-dead") }) @@ -2920,7 +2920,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) { req, _ := http.NewRequest("PUT", "/v1/agent/check/register", jsonReader(nodeCheck)) resp := httptest.NewRecorder() a.srv.h.ServeHTTP(resp, req) - require.Equal(t, http.StatusForbidden, resp.Code) + require.Equal(r, http.StatusForbidden, resp.Code) }) }) @@ -2930,7 +2930,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) { req.Header.Add("X-Consul-Token", svcToken.SecretID) resp := httptest.NewRecorder() a.srv.h.ServeHTTP(resp, req) - require.Equal(t, http.StatusForbidden, resp.Code) + require.Equal(r, http.StatusForbidden, resp.Code) }) }) @@ -2940,7 +2940,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) { req.Header.Add("X-Consul-Token", nodeToken.SecretID) resp := httptest.NewRecorder() a.srv.h.ServeHTTP(resp, req) - require.Equal(t, http.StatusOK, resp.Code) + require.Equal(r, http.StatusOK, resp.Code) }) }) @@ -2949,7 +2949,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) { req, _ := http.NewRequest("PUT", "/v1/agent/check/register", jsonReader(svcCheck)) resp := httptest.NewRecorder() a.srv.h.ServeHTTP(resp, req) - require.Equal(t, http.StatusForbidden, resp.Code) + require.Equal(r, http.StatusForbidden, resp.Code) }) }) @@ -2959,7 +2959,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) { req.Header.Add("X-Consul-Token", nodeToken.SecretID) resp := httptest.NewRecorder() a.srv.h.ServeHTTP(resp, req) - require.Equal(t, http.StatusForbidden, resp.Code) + require.Equal(r, http.StatusForbidden, resp.Code) }) }) @@ -2969,7 +2969,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) { req.Header.Add("X-Consul-Token", svcToken.SecretID) resp := httptest.NewRecorder() a.srv.h.ServeHTTP(resp, req) - require.Equal(t, http.StatusOK, resp.Code) + require.Equal(r, http.StatusOK, resp.Code) }) }) } @@ -5970,17 +5970,17 @@ func TestAgent_Monitor(t *testing.T) { res := httptest.NewRecorder() a.srv.h.ServeHTTP(res, registerReq) if http.StatusOK != res.Code { - t.Fatalf("expected 200 but got %v", res.Code) + r.Fatalf("expected 200 but got %v", res.Code) } // Wait until we have received some type of logging output - require.Eventually(t, func() bool { + require.Eventually(r, func() bool { return len(resp.Body.Bytes()) > 0 }, 3*time.Second, 100*time.Millisecond) cancelFunc() code := <-codeCh - require.Equal(t, http.StatusOK, code) + require.Equal(r, http.StatusOK, code) got := resp.Body.String() // Only check a substring that we are highly confident in finding @@ -6020,11 +6020,11 @@ func TestAgent_Monitor(t *testing.T) { res := httptest.NewRecorder() a.srv.h.ServeHTTP(res, registerReq) if http.StatusOK != res.Code { - t.Fatalf("expected 200 but got %v", res.Code) + r.Fatalf("expected 200 but got %v", res.Code) } // Wait until we have received some type of logging output - require.Eventually(t, func() bool { + require.Eventually(r, func() bool { return len(resp.Body.Bytes()) > 0 }, 3*time.Second, 100*time.Millisecond) cancelFunc() @@ -6057,24 +6057,24 @@ func TestAgent_Monitor(t *testing.T) { res := httptest.NewRecorder() a.srv.h.ServeHTTP(res, registerReq) if http.StatusOK != res.Code { - t.Fatalf("expected 200 but got %v", res.Code) + r.Fatalf("expected 200 but got %v", res.Code) } // Wait until we have received some type of logging output - require.Eventually(t, func() bool { + require.Eventually(r, func() bool { return len(resp.Body.Bytes()) > 0 }, 3*time.Second, 100*time.Millisecond) cancelFunc() code := <-codeCh - require.Equal(t, http.StatusOK, code) + require.Equal(r, http.StatusOK, code) // Each line is output as a separate JSON object, we grab the first and // make sure it can be unmarshalled. firstLine := bytes.Split(resp.Body.Bytes(), []byte("\n"))[0] var output map[string]interface{} if err := json.Unmarshal(firstLine, &output); err != nil { - t.Fatalf("err: %v", err) + r.Fatalf("err: %v", err) } }) }) @@ -6666,7 +6666,7 @@ func TestAgentConnectCARoots_list(t *testing.T) { dec := json.NewDecoder(resp.Body) value := &structs.IndexedCARoots{} - require.NoError(t, dec.Decode(value)) + require.NoError(r, dec.Decode(value)) if ca.ID != value.ActiveRootID { r.Fatalf("%s != %s", ca.ID, value.ActiveRootID) } @@ -7074,7 +7074,7 @@ func TestAgentConnectCALeafCert_goodNotLocal(t *testing.T) { dec := json.NewDecoder(resp.Body) issued2 := &structs.IssuedCert{} - require.NoError(t, dec.Decode(issued2)) + require.NoError(r, dec.Decode(issued2)) if issued.CertPEM == issued2.CertPEM { r.Fatalf("leaf has not updated") } @@ -7086,9 +7086,9 @@ func TestAgentConnectCALeafCert_goodNotLocal(t *testing.T) { } // Verify that the cert is signed by the new CA - requireLeafValidUnderCA(t, issued2, ca) + requireLeafValidUnderCA(r, issued2, ca) - require.NotEqual(t, issued, issued2) + require.NotEqual(r, issued, issued2) }) } } @@ -7465,11 +7465,11 @@ func TestAgentConnectCALeafCert_secondaryDC_good(t *testing.T) { // Try and sign again (note no index/wait arg since cache should update in // background even if we aren't actively blocking) a2.srv.h.ServeHTTP(resp, req) - require.Equal(t, http.StatusOK, resp.Code) + require.Equal(r, http.StatusOK, resp.Code) dec := json.NewDecoder(resp.Body) issued2 := &structs.IssuedCert{} - require.NoError(t, dec.Decode(issued2)) + require.NoError(r, dec.Decode(issued2)) if issued.CertPEM == issued2.CertPEM { r.Fatalf("leaf has not updated") } @@ -7481,9 +7481,9 @@ func TestAgentConnectCALeafCert_secondaryDC_good(t *testing.T) { } // Verify that the cert is signed by the new CA - requireLeafValidUnderCA(t, issued2, dc1_ca2) + requireLeafValidUnderCA(r, issued2, dc1_ca2) - require.NotEqual(t, issued, issued2) + require.NotEqual(r, issued, issued2) }) } @@ -7493,12 +7493,12 @@ func waitForActiveCARoot(t *testing.T, srv *HTTPHandlers, expect *structs.CARoot resp := httptest.NewRecorder() srv.h.ServeHTTP(resp, req) if http.StatusOK != resp.Code { - t.Fatalf("expected 200 but got %v", resp.Code) + r.Fatalf("expected 200 but got %v", resp.Code) } dec := json.NewDecoder(resp.Body) roots := &structs.IndexedCARoots{} - require.NoError(t, dec.Decode(roots)) + require.NoError(r, dec.Decode(roots)) var root *structs.CARoot for _, r := range roots.Roots { diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index 8bbf8379b531..2fb8df726e69 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -140,7 +140,7 @@ func TestACLEndpoint_ReplicationStatus(t *testing.T) { retry.Run(t, func(r *retry.R) { var status structs.ACLReplicationStatus err := msgpackrpc.CallWithCodec(codec, "ACL.ReplicationStatus", &getR, &status) - require.NoError(t, err) + require.NoError(r, err) require.True(r, status.Enabled) require.True(r, status.Running) @@ -217,7 +217,7 @@ func TestACLEndpoint_TokenRead(t *testing.T) { time.Sleep(200 * time.Millisecond) err := aclEp.TokenRead(&req, &resp) require.Error(r, err) - require.ErrorContains(t, err, "ACL not found") + require.ErrorContains(r, err, "ACL not found") require.Nil(r, resp.Token) }) }) diff --git a/agent/consul/client_test.go b/agent/consul/client_test.go index 5843c54e6127..e8b79f04dc6d 100644 --- a/agent/consul/client_test.go +++ b/agent/consul/client_test.go @@ -176,7 +176,7 @@ func TestClient_LANReap(t *testing.T) { retry.Run(t, func(r *retry.R) { require.Len(r, c1.LANMembersInAgentPartition(), 1) server := c1.router.FindLANServer() - require.Nil(t, server) + require.Nil(r, server) }) } diff --git a/agent/consul/multilimiter/multilimiter_test.go b/agent/consul/multilimiter/multilimiter_test.go index b64f95febdcc..ceb38a2b850e 100644 --- a/agent/consul/multilimiter/multilimiter_test.go +++ b/agent/consul/multilimiter/multilimiter_test.go @@ -88,7 +88,7 @@ func TestRateLimiterCleanup(t *testing.T) { retry.RunWith(&retry.Timer{Wait: 100 * time.Millisecond, Timeout: 2 * time.Second}, t, func(r *retry.R) { v, ok := limiters.Get(key) require.True(r, ok) - require.NotNil(t, v) + require.NotNil(r, v) }) time.Sleep(c.ReconcileCheckInterval) diff --git a/agent/consul/prepared_query_endpoint_test.go b/agent/consul/prepared_query_endpoint_test.go index ca459ddfd896..5e20d6976bfb 100644 --- a/agent/consul/prepared_query_endpoint_test.go +++ b/agent/consul/prepared_query_endpoint_test.go @@ -19,7 +19,6 @@ import ( msgpackrpc "github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc" "github.com/hashicorp/consul-net-rpc/net/rpc" - "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/connect" grpcexternal "github.com/hashicorp/consul/agent/grpc-external" @@ -1676,8 +1675,7 @@ func TestPreparedQuery_Execute(t *testing.T) { assert.Len(t, reply.Nodes, 0) }) - expectNodes := func(t *testing.T, query *structs.PreparedQueryRequest, reply *structs.PreparedQueryExecuteResponse, n int) { - t.Helper() + expectNodes := func(t require.TestingT, query *structs.PreparedQueryRequest, reply *structs.PreparedQueryExecuteResponse, n int) { assert.Len(t, reply.Nodes, n) assert.Equal(t, "dc1", reply.Datacenter) assert.Equal(t, 0, reply.Failovers) @@ -1685,8 +1683,7 @@ func TestPreparedQuery_Execute(t *testing.T) { assert.Equal(t, query.Query.DNS, reply.DNS) assert.True(t, reply.QueryMeta.KnownLeader) } - expectFailoverNodes := func(t *testing.T, query *structs.PreparedQueryRequest, reply *structs.PreparedQueryExecuteResponse, n int) { - t.Helper() + expectFailoverNodes := func(t require.TestingT, query *structs.PreparedQueryRequest, reply *structs.PreparedQueryExecuteResponse, n int) { assert.Len(t, reply.Nodes, n) assert.Equal(t, "dc2", reply.Datacenter) assert.Equal(t, 1, reply.Failovers) @@ -1695,8 +1692,7 @@ func TestPreparedQuery_Execute(t *testing.T) { assert.True(t, reply.QueryMeta.KnownLeader) } - expectFailoverPeerNodes := func(t *testing.T, query *structs.PreparedQueryRequest, reply *structs.PreparedQueryExecuteResponse, n int) { - t.Helper() + expectFailoverPeerNodes := func(t require.TestingT, query *structs.PreparedQueryRequest, reply *structs.PreparedQueryExecuteResponse, n int) { assert.Len(t, reply.Nodes, n) assert.Equal(t, "", reply.Datacenter) assert.Equal(t, acceptingPeerName, reply.PeerName) @@ -2509,13 +2505,13 @@ func TestPreparedQuery_Execute(t *testing.T) { } var reply structs.PreparedQueryExecuteResponse - require.NoError(t, msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply)) + require.NoError(r, msgpackrpc.CallWithCodec(codec1, "PreparedQuery.Execute", &req, &reply)) for _, node := range reply.Nodes { - assert.NotEqual(t, "node3", node.Node.Node) + assert.NotEqual(r, "node3", node.Node.Node) } - expectNodes(t, &query, &reply, 9) + expectNodes(r, &query, &reply, 9) }) }) } diff --git a/agent/dns_test.go b/agent/dns_test.go index 1cc74213890b..645626c18004 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -3186,7 +3186,7 @@ func TestDNS_ServiceLookup_WanTranslation(t *testing.T) { } var out struct{} - require.NoError(t, a2.RPC(context.Background(), "Catalog.Register", args, &out)) + require.NoError(r, a2.RPC(context.Background(), "Catalog.Register", args, &out)) }) // Look up the SRV record via service and prepared query. @@ -3514,11 +3514,11 @@ func TestDNS_CaseInsensitiveServiceLookup(t *testing.T) { retry.Run(t, func(r *retry.R) { in, _, err := c.Exchange(m, a.DNSAddr()) if err != nil { - t.Fatalf("err: %v", err) + r.Fatalf("err: %v", err) } if len(in.Answer) != 1 { - t.Fatalf("question %v, empty lookup: %#v", question, in) + r.Fatalf("question %v, empty lookup: %#v", question, in) } }) } diff --git a/agent/grpc-external/services/peerstream/stream_test.go b/agent/grpc-external/services/peerstream/stream_test.go index 887364b107e8..5f5d399af9e7 100644 --- a/agent/grpc-external/services/peerstream/stream_test.go +++ b/agent/grpc-external/services/peerstream/stream_test.go @@ -1045,7 +1045,7 @@ func TestStreamResources_Server_ServiceUpdates(t *testing.T) { require.Equal(r, mongo.Service.CompoundServiceName().String(), msg.GetResponse().ResourceID) var nodes pbpeerstream.ExportedService - require.NoError(t, msg.GetResponse().Resource.UnmarshalTo(&nodes)) + require.NoError(r, msg.GetResponse().Resource.UnmarshalTo(&nodes)) require.Len(r, nodes.Nodes, 1) }) }) @@ -1074,12 +1074,12 @@ func TestStreamResources_Server_ServiceUpdates(t *testing.T) { msg, err := client.RecvWithTimeout(100 * time.Millisecond) require.NoError(r, err) require.Equal(r, pbpeerstream.TypeURLExportedServiceList, msg.GetResponse().ResourceURL) - require.Equal(t, subExportedServiceList, msg.GetResponse().ResourceID) - require.Equal(t, pbpeerstream.Operation_OPERATION_UPSERT, msg.GetResponse().Operation) + require.Equal(r, subExportedServiceList, msg.GetResponse().ResourceID) + require.Equal(r, pbpeerstream.Operation_OPERATION_UPSERT, msg.GetResponse().Operation) var exportedServices pbpeerstream.ExportedServiceList - require.NoError(t, msg.GetResponse().Resource.UnmarshalTo(&exportedServices)) - require.Equal(t, []string{structs.ServiceName{Name: "mongo"}.String()}, exportedServices.Services) + require.NoError(r, msg.GetResponse().Resource.UnmarshalTo(&exportedServices)) + require.Equal(r, []string{structs.ServiceName{Name: "mongo"}.String()}, exportedServices.Services) }) }) @@ -1091,12 +1091,12 @@ func TestStreamResources_Server_ServiceUpdates(t *testing.T) { msg, err := client.RecvWithTimeout(100 * time.Millisecond) require.NoError(r, err) require.Equal(r, pbpeerstream.TypeURLExportedServiceList, msg.GetResponse().ResourceURL) - require.Equal(t, subExportedServiceList, msg.GetResponse().ResourceID) - require.Equal(t, pbpeerstream.Operation_OPERATION_UPSERT, msg.GetResponse().Operation) + require.Equal(r, subExportedServiceList, msg.GetResponse().ResourceID) + require.Equal(r, pbpeerstream.Operation_OPERATION_UPSERT, msg.GetResponse().Operation) var exportedServices pbpeerstream.ExportedServiceList - require.NoError(t, msg.GetResponse().Resource.UnmarshalTo(&exportedServices)) - require.Len(t, exportedServices.Services, 0) + require.NoError(r, msg.GetResponse().Resource.UnmarshalTo(&exportedServices)) + require.Len(r, exportedServices.Services, 0) }) }) } diff --git a/agent/local/state_test.go b/agent/local/state_test.go index 6052fa4784d0..fa4d1c6eeacc 100644 --- a/agent/local/state_test.go +++ b/agent/local/state_test.go @@ -1317,13 +1317,13 @@ func TestAgentAntiEntropy_Checks(t *testing.T) { chk.CreateIndex, chk.ModifyIndex = 0, 0 switch chk.CheckID { case "mysql": - require.Equal(t, chk, chk1) + require.Equal(r, chk, chk1) case "redis": - require.Equal(t, chk, chk2) + require.Equal(r, chk, chk2) case "web": - require.Equal(t, chk, chk3) + require.Equal(r, chk, chk3) case "cache": - require.Equal(t, chk, chk5) + require.Equal(r, chk, chk5) case "serfHealth": // ignore default: @@ -1353,9 +1353,9 @@ func TestAgentAntiEntropy_Checks(t *testing.T) { addrs := services.NodeServices.Node.TaggedAddresses meta := services.NodeServices.Node.Meta delete(meta, structs.MetaSegmentKey) // Added later, not in config. - assert.Equal(t, a.Config.NodeID, id) - assert.Equal(t, a.Config.TaggedAddresses, addrs) - assert.Equal(t, unNilMap(a.Config.NodeMeta), meta) + assert.Equal(r, a.Config.NodeID, id) + assert.Equal(r, a.Config.TaggedAddresses, addrs) + assert.Equal(r, unNilMap(a.Config.NodeMeta), meta) } }) retry.Run(t, func(r *retry.R) { @@ -1382,11 +1382,11 @@ func TestAgentAntiEntropy_Checks(t *testing.T) { chk.CreateIndex, chk.ModifyIndex = 0, 0 switch chk.CheckID { case "mysql": - require.Equal(t, chk1, chk) + require.Equal(r, chk1, chk) case "web": - require.Equal(t, chk3, chk) + require.Equal(r, chk3, chk) case "cache": - require.Equal(t, chk5, chk) + require.Equal(r, chk5, chk) case "serfHealth": // ignore default: