From 12aacec76cdc8d77127ee7bb1b84b911f265c9d3 Mon Sep 17 00:00:00 2001
From: irfan sharif <irfanmahmoudsharif@gmail.com>
Date: Thu, 24 Sep 2020 18:03:14 -0400
Subject: [PATCH] kvserver: remove all test instances of empty liveness records

In a future commit we'll introduce assertions that this is no longer
possible. Let's update our tests to be more representative of inputs
we'd normally expect.

Release note: None
---
 pkg/kv/kvserver/node_liveness_test.go      | 34 ++++++++++++++++++----
 pkg/kv/kvserver/node_liveness_unit_test.go |  2 +-
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/pkg/kv/kvserver/node_liveness_test.go b/pkg/kv/kvserver/node_liveness_test.go
index ff48dfe163aa..32ee16e991dc 100644
--- a/pkg/kv/kvserver/node_liveness_test.go
+++ b/pkg/kv/kvserver/node_liveness_test.go
@@ -41,6 +41,7 @@ import (
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/logtags"
 	"github.com/gogo/protobuf/proto"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sync/errgroup"
 )
@@ -481,9 +482,10 @@ func TestNodeLivenessRestart(t *testing.T) {
 	// seeing the liveness record properly gossiped at store startup.
 	var expKeys []string
 	for _, g := range mtc.gossips {
-		key := gossip.MakeNodeLivenessKey(g.NodeID.Get())
+		nodeID := g.NodeID.Get()
+		key := gossip.MakeNodeLivenessKey(nodeID)
 		expKeys = append(expKeys, key)
-		if err := g.AddInfoProto(key, &kvserverpb.Liveness{}, 0); err != nil {
+		if err := g.AddInfoProto(key, &kvserverpb.Liveness{NodeID: nodeID}, 0); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -612,7 +614,15 @@ func TestNodeLivenessGetIsLiveMap(t *testing.T) {
 
 	// Advance the clock but only heartbeat node 0.
 	mtc.manualClock.Increment(mtc.nodeLivenesses[0].GetLivenessThreshold().Nanoseconds() + 1)
-	liveness, _ := mtc.nodeLivenesses[0].GetLiveness(mtc.gossips[0].NodeID.Get())
+	var liveness kvserver.LivenessRecord
+	testutils.SucceedsSoon(t, func() error {
+		livenessRec, err := mtc.nodeLivenesses[0].GetLiveness(mtc.gossips[0].NodeID.Get())
+		if err != nil {
+			return err
+		}
+		liveness = livenessRec
+		return nil
+	})
 
 	testutils.SucceedsSoon(t, func() error {
 		if err := mtc.nodeLivenesses[0].Heartbeat(context.Background(), liveness.Liveness); err != nil {
@@ -668,7 +678,15 @@ func TestNodeLivenessGetLivenesses(t *testing.T) {
 
 	// Advance the clock but only heartbeat node 0.
 	mtc.manualClock.Increment(mtc.nodeLivenesses[0].GetLivenessThreshold().Nanoseconds() + 1)
-	liveness, _ := mtc.nodeLivenesses[0].GetLiveness(mtc.gossips[0].NodeID.Get())
+	var liveness kvserver.LivenessRecord
+	testutils.SucceedsSoon(t, func() error {
+		livenessRec, err := mtc.nodeLivenesses[0].GetLiveness(mtc.gossips[0].NodeID.Get())
+		if err != nil {
+			return err
+		}
+		liveness = livenessRec
+		return nil
+	})
 	if err := mtc.nodeLivenesses[0].Heartbeat(context.Background(), liveness.Liveness); err != nil {
 		t.Fatal(err)
 	}
@@ -791,7 +809,9 @@ func TestNodeLivenessSetDraining(t *testing.T) {
 	// Verify success on failed update of a liveness record that already has the
 	// given draining setting.
 	if err := mtc.nodeLivenesses[drainingNodeIdx].SetDrainingInternal(
-		ctx, kvserver.LivenessRecord{}, false,
+		ctx, kvserver.LivenessRecord{Liveness: kvserverpb.Liveness{
+			NodeID: drainingNodeID,
+		}}, false,
 	); err != nil {
 		t.Fatal(err)
 	}
@@ -1079,8 +1099,10 @@ func testNodeLivenessSetDecommissioning(t *testing.T, decommissionNodeIdx int) {
 
 	// Verify success on failed update of a liveness record that already has the
 	// given decommissioning setting.
+	oldLivenessRec, err := callerNodeLiveness.GetLiveness(nodeID)
+	assert.Nil(t, err)
 	if _, err := callerNodeLiveness.SetDecommissioningInternal(
-		ctx, nodeID, kvserver.LivenessRecord{}, kvserverpb.MembershipStatus_ACTIVE,
+		ctx, nodeID, oldLivenessRec, kvserverpb.MembershipStatus_ACTIVE,
 	); err != nil {
 		t.Fatal(err)
 	}
diff --git a/pkg/kv/kvserver/node_liveness_unit_test.go b/pkg/kv/kvserver/node_liveness_unit_test.go
index 17fc439995dc..412e69262953 100644
--- a/pkg/kv/kvserver/node_liveness_unit_test.go
+++ b/pkg/kv/kvserver/node_liveness_unit_test.go
@@ -58,8 +58,8 @@ func TestShouldReplaceLiveness(t *testing.T) {
 	}{
 		{
 			// Epoch update only.
-			kvserverpb.Liveness{},
 			l(1, hlc.Timestamp{}, false, "active"),
+			l(2, hlc.Timestamp{}, false, "active"),
 			yes,
 		},
 		{