From 59dbd070d7c632675af950f69e6278e419cd3a89 Mon Sep 17 00:00:00 2001
From: Freddy <freddygv@users.noreply.github.com>
Date: Wed, 17 Jul 2019 09:33:38 -0600
Subject: [PATCH] More flaky test fixes (#6151)

* Add retry to TestAPI_ClientTxn

* Add retry to TestLeader_RegisterMember

* Account for empty watch result in ConnectRootsWatch
---
 agent/consul/leader_test.go |  16 +++---
 api/txn_test.go             | 111 ++++++++++++++++++------------------
 api/watch/funcs_test.go     |   2 +-
 3 files changed, 66 insertions(+), 63 deletions(-)

diff --git a/agent/consul/leader_test.go b/agent/consul/leader_test.go
index 7372bef730..e394722885 100644
--- a/agent/consul/leader_test.go
+++ b/agent/consul/leader_test.go
@@ -68,13 +68,15 @@ func TestLeader_RegisterMember(t *testing.T) {
 	}
 
 	// Server should be registered
-	_, node, err := state.GetNode(s1.config.NodeName)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if node == nil {
-		t.Fatalf("server not registered")
-	}
+	retry.Run(t, func(r *retry.R) {
+		_, node, err := state.GetNode(s1.config.NodeName)
+		if err != nil {
+			r.Fatalf("err: %v", err)
+		}
+		if node == nil {
+			r.Fatalf("server not registered")
+		}
+	})
 
 	// Service should be registered
 	_, services, err := state.NodeServices(nil, s1.config.NodeName)
diff --git a/api/txn_test.go b/api/txn_test.go
index e2106d7475..b940546d19 100644
--- a/api/txn_test.go
+++ b/api/txn_test.go
@@ -1,19 +1,18 @@
 package api
 
 import (
+	"github.com/hashicorp/consul/sdk/testutil/retry"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/hashicorp/go-uuid"
 
-	"github.com/pascaldekloe/goe/verify"
 	"github.com/stretchr/testify/require"
 )
 
 func TestAPI_ClientTxn(t *testing.T) {
 	t.Parallel()
-	require := require.New(t)
 	c, s := makeClient(t)
 	defer s.Stop()
 
@@ -24,7 +23,7 @@ func TestAPI_ClientTxn(t *testing.T) {
 
 	// Set up a test service and health check.
 	nodeID, err := uuid.GenerateUUID()
-	require.NoError(err)
+	require.NoError(t, err)
 
 	catalog := c.Catalog()
 	reg := &CatalogRegistration{
@@ -59,11 +58,11 @@ func TestAPI_ClientTxn(t *testing.T) {
 		},
 	}
 	_, err = catalog.Register(reg, nil)
-	require.NoError(err)
+	require.NoError(t, err)
 
 	node, _, err := catalog.Node("foo", nil)
-	require.NoError(err)
-	require.Equal(nodeID, node.Node.ID)
+	require.NoError(t, err)
+	require.Equal(t, nodeID, node.Node.ID)
 
 	// Make a session.
 	id, _, err := session.CreateNoChecks(nil, nil)
@@ -218,58 +217,60 @@ func TestAPI_ClientTxn(t *testing.T) {
 			},
 		},
 	}
-	verify.Values(t, "", ret.Results, expected)
+	require.Equal(t, ret.Results, expected)
 
-	// Run a read-only transaction.
-	ops = TxnOps{
-		&TxnOp{
-			KV: &KVTxnOp{
-				Verb: KVGet,
-				Key:  key,
-			},
-		},
-		&TxnOp{
-			Node: &NodeTxnOp{
-				Verb: NodeGet,
-				Node: Node{ID: s.Config.NodeID, Node: s.Config.NodeName},
-			},
-		},
-	}
-	ok, ret, _, err = txn.Txn(ops, nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	} else if !ok {
-		t.Fatalf("transaction failure")
-	}
-
-	expected = TxnResults{
-		&TxnResult{
-			KV: &KVPair{
-				Key:         key,
-				Session:     id,
-				Value:       []byte("test"),
-				LockIndex:   1,
-				CreateIndex: ret.Results[0].KV.CreateIndex,
-				ModifyIndex: ret.Results[0].KV.ModifyIndex,
-			},
-		},
-		&TxnResult{
-			Node: &Node{
-				ID:         s.Config.NodeID,
-				Node:       s.Config.NodeName,
-				Address:    "127.0.0.1",
-				Datacenter: "dc1",
-				TaggedAddresses: map[string]string{
-					"lan": s.Config.Bind,
-					"wan": s.Config.Bind,
+	retry.Run(t, func(r *retry.R) {
+		// Run a read-only transaction.
+		ops = TxnOps{
+			&TxnOp{
+				KV: &KVTxnOp{
+					Verb: KVGet,
+					Key:  key,
 				},
-				Meta:        map[string]string{"consul-network-segment": ""},
-				CreateIndex: ret.Results[1].Node.CreateIndex,
-				ModifyIndex: ret.Results[1].Node.ModifyIndex,
 			},
-		},
-	}
-	verify.Values(t, "", ret.Results, expected)
+			&TxnOp{
+				Node: &NodeTxnOp{
+					Verb: NodeGet,
+					Node: Node{ID: s.Config.NodeID, Node: s.Config.NodeName},
+				},
+			},
+		}
+		ok, ret, _, err = txn.Txn(ops, nil)
+		if err != nil {
+			r.Fatalf("err: %v", err)
+		} else if !ok {
+			r.Fatalf("transaction failure")
+		}
+
+		expected = TxnResults{
+			&TxnResult{
+				KV: &KVPair{
+					Key:         key,
+					Session:     id,
+					Value:       []byte("test"),
+					LockIndex:   1,
+					CreateIndex: ret.Results[0].KV.CreateIndex,
+					ModifyIndex: ret.Results[0].KV.ModifyIndex,
+				},
+			},
+			&TxnResult{
+				Node: &Node{
+					ID:         s.Config.NodeID,
+					Node:       s.Config.NodeName,
+					Address:    "127.0.0.1",
+					Datacenter: "dc1",
+					TaggedAddresses: map[string]string{
+						"lan": s.Config.Bind,
+						"wan": s.Config.Bind,
+					},
+					Meta:        map[string]string{"consul-network-segment": ""},
+					CreateIndex: ret.Results[1].Node.CreateIndex,
+					ModifyIndex: ret.Results[1].Node.ModifyIndex,
+				},
+			},
+		}
+		require.Equal(r, ret.Results, expected)
+	})
 
 	// Sanity check using the regular GET API.
 	kv := c.KV()
diff --git a/api/watch/funcs_test.go b/api/watch/funcs_test.go
index be70f591c2..5e7091e23b 100644
--- a/api/watch/funcs_test.go
+++ b/api/watch/funcs_test.go
@@ -715,7 +715,7 @@ func TestConnectRootsWatch(t *testing.T) {
 			return // ignore
 		}
 		v, ok := raw.(*api.CARootList)
-		if !ok || v == nil {
+		if !ok || v == nil || len(v.Roots) == 0 {
 			return // ignore
 		}
 		// Only 1 CA is the bootstrapped state (i.e. first response). Ignore this