Skip to content

Commit

Permalink
fix some testing.T retry.R mixups (#17600) (#17601)
Browse files Browse the repository at this point in the history 
backport of m #17600 onto release/1.13.x

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
  • Loading branch information
@hc-github-team-consul-core @rboyer
hc-github-team-consul-core and rboyer authored Jun 7, 2023
1 parent 3a85cf8 commit 9a4de8a
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 49 deletions.
56 changes: 28 additions & 28 deletions agent/agent_endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1781,7 +1781,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 != "" {
Expand Down Expand Up @@ -1868,14 +1868,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")
})
Expand Down Expand Up @@ -2864,7 +2864,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)
})
})

Expand All @@ -2873,7 +2873,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) {
req, _ := http.NewRequest("PUT", "/v1/agent/check/register?token="+svcToken.SecretID, 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)
})
})

Expand All @@ -2882,7 +2882,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) {
req, _ := http.NewRequest("PUT", "/v1/agent/check/register?token="+nodeToken.SecretID, jsonReader(nodeCheck))
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
require.Equal(t, http.StatusOK, resp.Code)
require.Equal(r, http.StatusOK, resp.Code)
})
})

Expand All @@ -2891,7 +2891,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)
})
})

Expand All @@ -2900,7 +2900,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) {
req, _ := http.NewRequest("PUT", "/v1/agent/check/register?token="+nodeToken.SecretID, 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)
})
})

Expand All @@ -2909,7 +2909,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) {
req, _ := http.NewRequest("PUT", "/v1/agent/check/register?token="+svcToken.SecretID, jsonReader(svcCheck))
resp := httptest.NewRecorder()
a.srv.h.ServeHTTP(resp, req)
require.Equal(t, http.StatusOK, resp.Code)
require.Equal(r, http.StatusOK, resp.Code)
})
})
}
Expand Down Expand Up @@ -5892,17 +5892,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
Expand Down Expand Up @@ -5942,11 +5942,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()
Expand Down Expand Up @@ -5979,24 +5979,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)
}
})
})
Expand Down Expand Up @@ -6557,7 +6557,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)
}
Expand Down Expand Up @@ -6960,7 +6960,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")
}
Expand All @@ -6972,9 +6972,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)
})
}
}
Expand Down Expand Up @@ -7351,11 +7351,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")
}
Expand All @@ -7367,9 +7367,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)
})
}

Expand All @@ -7379,12 +7379,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 {
Expand Down
4 changes: 2 additions & 2 deletions agent/consul/acl_endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,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/consul/authmethod/kubeauth"
"github.com/hashicorp/consul/agent/consul/authmethod/testauth"
Expand Down Expand Up @@ -93,7 +92,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)
Expand Down Expand Up @@ -5625,6 +5624,7 @@ func deleteTestAuthMethod(codec rpc.ClientCodec, initialManagementToken string,
err := msgpackrpc.CallWithCodec(codec, "ACL.AuthMethodDelete", &arg, &ignored)
return err
}

func upsertTestAuthMethod(
codec rpc.ClientCodec, initialManagementToken string, datacenter string,
sessionID string,
Expand Down
2 changes: 1 addition & 1 deletion agent/consul/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,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)
})
}

Expand Down
9 changes: 6 additions & 3 deletions agent/dns_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ func TestDNSCycleRecursorCheck(t *testing.T) {
}
require.Equal(t, wantAnswer, in.Answer)
}

func TestDNSCycleRecursorCheckAllFail(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
Expand Down Expand Up @@ -511,6 +512,7 @@ func TestDNSCycleRecursorCheckAllFail(t *testing.T) {
// Verify if we hit SERVFAIL from Consul
require.Equal(t, dns.RcodeServerFailure, in.Rcode)
}

func TestDNS_NodeLookup_CNAME(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
Expand Down Expand Up @@ -3176,7 +3178,7 @@ func TestDNS_ServiceLookup_WanTranslation(t *testing.T) {
}

var out struct{}
require.NoError(t, a2.RPC("Catalog.Register", args, &out))
require.NoError(r, a2.RPC("Catalog.Register", args, &out))
})

// Look up the SRV record via service and prepared query.
Expand Down Expand Up @@ -3504,11 +3506,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)
}
})
}
Expand Down Expand Up @@ -6355,6 +6357,7 @@ func TestDNS_ServiceLookup_FilterACL(t *testing.T) {
})
}
}

func TestDNS_ServiceLookup_MetaTXT(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
Expand Down
10 changes: 5 additions & 5 deletions agent/grpc-external/services/peerstream/stream_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,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)
})
})
Expand Down Expand Up @@ -1214,8 +1214,8 @@ func TestStreamResources_Server_SendsHeartbeats(t *testing.T) {
Wait: outgoingHeartbeatInterval / 2,
}, t, func(r *retry.R) {
heartbeat, err := client.Recv()
require.NoError(t, err)
require.NotNil(t, heartbeat.GetHeartbeat())
require.NoError(r, err)
require.NotNil(r, heartbeat.GetHeartbeat())
})
})

Expand All @@ -1225,8 +1225,8 @@ func TestStreamResources_Server_SendsHeartbeats(t *testing.T) {
Wait: outgoingHeartbeatInterval / 2,
}, t, func(r *retry.R) {
heartbeat, err := client.Recv()
require.NoError(t, err)
require.NotNil(t, heartbeat.GetHeartbeat())
require.NoError(r, err)
require.NotNil(r, heartbeat.GetHeartbeat())
})
})
}
Expand Down
20 changes: 10 additions & 10 deletions agent/local/state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1130,13 +1130,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:
Expand Down Expand Up @@ -1166,9 +1166,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) {
Expand All @@ -1195,11 +1195,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:
Expand Down

0 comments on commit 9a4de8a

Please sign in to comment.