Skip to content

Commit

Permalink
wrap flaky tests with t.Run; expose the retry.R
Browse files Browse the repository at this point in the history
  • Loading branch information
@nfi-hashicorp
nfi-hashicorp committed Aug 8, 2023
1 parent 4f3aea1 commit c656dbf
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 25 deletions.
53 changes: 33 additions & 20 deletions agent/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/hashicorp/consul/agent/rpc/middleware"
"github.com/hashicorp/consul/lib/retry"
"github.com/hashicorp/consul/sdk/testutil"
testutilretry "github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/hashicorp/consul/testrpc"
"github.com/hashicorp/consul/tlsutil"
)
Expand Down Expand Up @@ -174,11 +175,11 @@ func assertMetricNotExists(t *testing.T, respRec *httptest.ResponseRecorder, met
// TestAgent_OneTwelveRPCMetrics test for the 1.12 style RPC metrics. These are the labeled metrics coming from
// agent.rpc.middleware.interceptors package.
func TestAgent_OneTwelveRPCMetrics(t *testing.T) {
testutil.RetryFlakyTest(t, func() {
skipIfShortTesting(t)
// This test cannot use t.Parallel() since we modify global state, ie the global metrics instance
skipIfShortTesting(t)
// This test cannot use t.Parallel() since we modify global state, ie the global metrics instance

t.Run("Check that 1.12 rpc metrics are not emitted by default.", func(t *testing.T) {
t.Run("Check that 1.12 rpc metrics are not emitted by default.", func(t *testing.T) {
testutil.RetryFlakyTest(t, func(r *testutilretry.R) {
metricsPrefix := "new_rpc_metrics"
hcl := fmt.Sprintf(`
telemetry = {
Expand All @@ -193,15 +194,17 @@ func TestAgent_OneTwelveRPCMetrics(t *testing.T) {

var out struct{}
err := a.RPC(context.Background(), "Status.Ping", struct{}{}, &out)
require.NoError(t, err)
require.NoError(r, err)

respRec := httptest.NewRecorder()
recordPromMetrics(t, a, respRec)

assertMetricNotExists(t, respRec, metricsPrefix+"_rpc_server_call")
})
})

t.Run("Check that 1.12 rpc metrics are emitted when specified by operator.", func(t *testing.T) {
t.Run("Check that 1.12 rpc metrics are emitted when specified by operator.", func(t *testing.T) {
testutil.RetryFlakyTest(t, func(r *testutilretry.R) {
metricsPrefix := "new_rpc_metrics_2"
allowRPCMetricRule := metricsPrefix + "." + strings.Join(middleware.OneTwelveRPCSummary[0].Name, ".")
hcl := fmt.Sprintf(`
Expand Down Expand Up @@ -439,11 +442,11 @@ func TestHTTPHandlers_AgentMetrics_CACertExpiry_Prometheus(t *testing.T) {
}

func TestHTTPHandlers_AgentMetrics_WAL_Prometheus(t *testing.T) {
testutil.RetryFlakyTest(t, func() {
skipIfShortTesting(t)
// This test cannot use t.Parallel() since we modify global state, ie the global metrics instance
skipIfShortTesting(t)
// This test cannot use t.Parallel() since we modify global state, ie the global metrics instance

t.Run("client agent emits nothing", func(t *testing.T) {
t.Run("client agent emits nothing", func(t *testing.T) {
testutil.RetryFlakyTest(t, func(r *testutilretry.R) {
hcl := `
server = false
telemetry = {
Expand All @@ -463,10 +466,13 @@ func TestHTTPHandlers_AgentMetrics_WAL_Prometheus(t *testing.T) {
respRec := httptest.NewRecorder()
recordPromMetrics(t, a, respRec)

require.NotContains(t, respRec.Body.String(), "agent_4_raft_wal")
require.NotContains(r, respRec.Body.String(), "agent_4_raft_wal")
})
})

t.Run("server with WAL enabled emits WAL metrics", func(t *testing.T) {
testutil.RetryFlakyTest(t, func(r *testutilretry.R) {

t.Run("server with WAL enabled emits WAL metrics", func(t *testing.T) {
hcl := `
server = true
bootstrap = true
Expand Down Expand Up @@ -503,8 +509,11 @@ func TestHTTPHandlers_AgentMetrics_WAL_Prometheus(t *testing.T) {
require.Contains(t, out, "agent_5_raft_wal_stable_sets")
require.Contains(t, out, "agent_5_raft_wal_tail_truncations")
})
})

t.Run("server without WAL enabled emits no WAL metrics", func(t *testing.T) {
testutil.RetryFlakyTest(t, func(r *testutilretry.R) {

t.Run("server without WAL enabled emits no WAL metrics", func(t *testing.T) {
hcl := `
server = true
bootstrap = true
Expand Down Expand Up @@ -535,11 +544,12 @@ func TestHTTPHandlers_AgentMetrics_WAL_Prometheus(t *testing.T) {
}

func TestHTTPHandlers_AgentMetrics_LogVerifier_Prometheus(t *testing.T) {
testutil.RetryFlakyTest(t, func() {
skipIfShortTesting(t)
// This test cannot use t.Parallel() since we modify global state, ie the global metrics instance
skipIfShortTesting(t)
// This test cannot use t.Parallel() since we modify global state, ie the global metrics instance

t.Run("client agent emits nothing", func(t *testing.T) {
testutil.RetryFlakyTest(t, func(r *testutilretry.R) {

t.Run("client agent emits nothing", func(t *testing.T) {
hcl := `
server = false
telemetry = {
Expand All @@ -564,8 +574,10 @@ func TestHTTPHandlers_AgentMetrics_LogVerifier_Prometheus(t *testing.T) {

require.NotContains(t, respRec.Body.String(), "agent_4_raft_logstore_verifier")
})
})

t.Run("server with verifier enabled emits all metrics", func(t *testing.T) {
t.Run("server with verifier enabled emits all metrics", func(t *testing.T) {
testutil.RetryFlakyTest(t, func(r *testutilretry.R) {
hcl := `
server = true
bootstrap = true
Expand Down Expand Up @@ -599,8 +611,10 @@ func TestHTTPHandlers_AgentMetrics_LogVerifier_Prometheus(t *testing.T) {
require.Contains(t, out, "agent_5_raft_logstore_verifier_read_checksum_failures")
require.Contains(t, out, "agent_5_raft_logstore_verifier_write_checksum_failures")
})
})

t.Run("server with verifier disabled emits no extra metrics", func(t *testing.T) {
t.Run("server with verifier disabled emits no extra metrics", func(t *testing.T) {
testutil.RetryFlakyTest(t, func(r *testutilretry.R) {
hcl := `
server = true
bootstrap = true
Expand Down Expand Up @@ -628,6 +642,5 @@ func TestHTTPHandlers_AgentMetrics_LogVerifier_Prometheus(t *testing.T) {

require.NotContains(t, respRec.Body.String(), "agent_6_raft_logstore_verifier")
})

})
}
3 changes: 2 additions & 1 deletion command/acl/token/update/token_update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/hashicorp/consul/agent"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/sdk/testutil"
testutilretry "github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/hashicorp/consul/testrpc"
)

Expand Down Expand Up @@ -183,7 +184,7 @@ func TestTokenUpdateCommand(t *testing.T) {
}

func TestTokenUpdateCommandWithAppend(t *testing.T) {
testutil.RetryFlakyTest(t, func() {
testutil.RetryFlakyTest(t, func(_ *testutilretry.R) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}
Expand Down
3 changes: 2 additions & 1 deletion command/operator/usage/instances/usage_instances_oss_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/sdk/testutil"
testutilretry "github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -120,7 +121,7 @@ Total 45`,
}

func TestUsageInstances_formatNodesCounts(t *testing.T) {
testutil.RetryFlakyTest(t, func() {
testutil.RetryFlakyTest(t, func(_ *testutilretry.R) {
usageBasic := map[string]api.ServiceUsage{
"dc1": {
Nodes: 10,
Expand Down
10 changes: 7 additions & 3 deletions sdk/testutil/retryflakytest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ func (f *Forever) Continue() bool {
}

// RetryFlakyTest retries a flaky test forever
func RetryFlakyTest(t *testing.T, f func()) {
retry.RunWith(&Forever{}, t, func(_ *retry.R) {
f()
func RetryFlakyTest(t *testing.T, f func(*retry.R)) {
i := 0
retry.RunWith(&Forever{}, t, func(r *retry.R) {
t.Logf("RetryFlakyTest: %s %d", t.Name(), i)
// wrap with a t.Run to contain t.Fail and friends
t.Run("retry", func(t *testing.T) { f(r) })
i++
})
}

0 comments on commit c656dbf

Please sign in to comment.