test: Move client server manager tests to use must library. (#25569)

client/servers/manager_internal_test.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/hashicorp/nomad/ci"
 	"github.com/hashicorp/nomad/helper/testlog"
+	"github.com/shoenig/test/must"
 )
 
 type fauxAddr struct {
@@ -41,13 +42,6 @@ func testManager(t *testing.T) (m *Manager) {
 	return m
 }
 
-func testManagerFailProb(t *testing.T, failPct float64) (m *Manager) {
-	logger := testlog.HCLogger(t)
-	shutdownCh := make(chan struct{})
-	m = New(logger, shutdownCh, &fauxConnPool{failPct: failPct})
-	return m
-}
-
 func TestManagerInternal_cycleServer(t *testing.T) {
 	ci.Parallel(t)
 
@@ -57,45 +51,29 @@ func TestManagerInternal_cycleServer(t *testing.T) {
 	srvs := Servers([]*Server{server0, server1, server2})
 
 	srvs.cycle()
-	if len(srvs) != 3 {
+	must.Eq(t, len(srvs), 3)
-		t.Fatalf("server length incorrect: %d/3", len(srvs))
+	must.SliceEqual(t, []*Server{server1, server2, server0}, srvs, must.Sprint(
-	}
+		"server ordering after one cycle not correct"),
-	if srvs[0] != server1 &&
+	)
-		srvs[1] != server2 &&
-		srvs[2] != server0 {
-		t.Fatalf("server ordering after one cycle not correct")
-	}
 
 	srvs.cycle()
-	if srvs[0] != server2 &&
+	must.SliceEqual(t, []*Server{server2, server0, server1}, srvs, must.Sprint(
-		srvs[1] != server0 &&
+		"server ordering after two cycles not correct"),
-		srvs[2] != server1 {
+	)
-		t.Fatalf("server ordering after two cycles not correct")
-	}
 
 	srvs.cycle()
-	if srvs[0] != server0 &&
+	must.SliceEqual(t, []*Server{server0, server1, server2}, srvs, must.Sprint(
-		srvs[1] != server1 &&
+		"server ordering after three cycles not correct"),
-		srvs[2] != server2 {
+	)
-		t.Fatalf("server ordering after three cycles not correct")
-	}
 }
 
 func TestManagerInternal_New(t *testing.T) {
 	ci.Parallel(t)
 
 	m := testManager(t)
-	if m == nil {
+	must.NotNil(t, m)
-		t.Fatalf("Manager nil")
+	must.NotNil(t, m.logger)
-	}
+	must.NotNil(t, m.shutdownCh)
-
-	if m.logger == nil {
-		t.Fatalf("Manager.logger nil")
-	}
-
-	if m.shutdownCh == nil {
-		t.Fatalf("Manager.shutdownCh nil")
-	}
 }
 
 // func (l *serverList) refreshServerRebalanceTimer() {
@@ -156,8 +134,8 @@ func TestManagerInternal_refreshServerRebalanceTimer(t *testing.T) {
 
 		d := m.refreshServerRebalanceTimer()
 		t.Logf("Nodes: %d; Servers: %d; Refresh: %v; Min: %v", s.numNodes, s.numServers, d, s.minRebalance)
-		if d < s.minRebalance {
+		must.Greater(t, s.minRebalance, d, must.Sprintf(
-			t.Errorf("duration too short for cluster of size %d and %d servers (%s < %s)", s.numNodes, s.numServers, d, s.minRebalance)
+			"duration too short for cluster of size %d and %d servers (%s < %s)", s.numNodes, s.numServers, d, s.minRebalance,
-		}
+		))
 	}
 }

client/servers/manager_test.go

@@ -13,7 +13,7 @@ import (
 	"github.com/hashicorp/nomad/ci"
 	"github.com/hashicorp/nomad/client/servers"
 	"github.com/hashicorp/nomad/helper/testlog"
-	"github.com/stretchr/testify/require"
+	"github.com/shoenig/test/must"
 )
 
 type fauxAddr struct {
@@ -53,38 +53,32 @@ func testManagerFailProb(t *testing.T, failPct float64) (m *servers.Manager) {
 func TestServers_SetServers(t *testing.T) {
 	ci.Parallel(t)
 
-	require := require.New(t)
 	m := testManager(t)
-	var num int
+	must.Zero(t, m.NumServers())
-	num = m.NumServers()
-	if num != 0 {
-		t.Fatalf("Expected zero servers to start")
-	}
 
 	s1 := &servers.Server{Addr: &fauxAddr{"server1"}}
 	s2 := &servers.Server{Addr: &fauxAddr{"server2"}}
-	require.True(m.SetServers([]*servers.Server{s1, s2}))
+	must.True(t, m.SetServers([]*servers.Server{s1, s2}))
-	require.False(m.SetServers([]*servers.Server{s1, s2}))
+	must.False(t, m.SetServers([]*servers.Server{s1, s2}))
-	require.False(m.SetServers([]*servers.Server{s2, s1}))
+	must.False(t, m.SetServers([]*servers.Server{s2, s1}))
-	require.Equal(2, m.NumServers())
+	must.Eq(t, 2, m.NumServers())
-	require.Len(m.GetServers(), 2)
+	must.Len(t, 2, m.GetServers())
 
-	require.True(m.SetServers([]*servers.Server{s1}))
+	must.True(t, m.SetServers([]*servers.Server{s1}))
-	require.Equal(1, m.NumServers())
+	must.Eq(t, 1, m.NumServers())
-	require.Len(m.GetServers(), 1)
+	must.Len(t, 1, m.GetServers())
 
 	// Test that the list of servers does not get shuffled
 	// as a side effect when incoming list is equal
-	require.True(m.SetServers([]*servers.Server{s1, s2}))
+	must.True(t, m.SetServers([]*servers.Server{s1, s2}))
 	before := m.GetServers()
-	require.False(m.SetServers([]*servers.Server{s1, s2}))
+	must.False(t, m.SetServers([]*servers.Server{s1, s2}))
 	after := m.GetServers()
-	require.Equal(before, after)
+	must.Equal(t, before, after)
 
 	// Send a shuffled list, verify original order doesn't change
-	require.False(m.SetServers([]*servers.Server{s2, s1}))
+	must.False(t, m.SetServers([]*servers.Server{s2, s1}))
-	afterShuffledInput := m.GetServers()
+	must.Equal(t, after, m.GetServers())
-	require.Equal(after, afterShuffledInput)
 }
 
 func TestServers_FindServer(t *testing.T) {
@@ -92,35 +86,25 @@ func TestServers_FindServer(t *testing.T) {
 
 	m := testManager(t)
 
-	if m.FindServer() != nil {
+	must.Nil(t, m.FindServer())
-		t.Fatalf("Expected nil return")
-	}
 
 	var srvs []*servers.Server
 	srvs = append(srvs, &servers.Server{Addr: &fauxAddr{"s1"}})
 	m.SetServers(srvs)
-	if m.NumServers() != 1 {
+	must.Eq(t, 1, m.NumServers())
-		t.Fatalf("Expected one server")
-	}
 
 	s1 := m.FindServer()
-	if s1 == nil {
+	must.NotNil(t, s1)
-		t.Fatalf("Expected non-nil server")
+	must.StrEqFold(t, "s1", s1.String())
-	}
-	if s1.String() != "s1" {
-		t.Fatalf("Expected s1 server")
-	}
 
 	s1 = m.FindServer()
-	if s1 == nil || s1.String() != "s1" {
+	must.NotNil(t, s1)
-		t.Fatalf("Expected s1 server (still)")
+	must.StrEqFold(t, "s1", s1.String())
-	}
 
 	srvs = append(srvs, &servers.Server{Addr: &fauxAddr{"s2"}})
 	m.SetServers(srvs)
-	if m.NumServers() != 2 {
+	must.Eq(t, 2, m.NumServers())
-		t.Fatalf("Expected two servers")
+
-	}
 	s1 = m.FindServer()
 
 	for _, srv := range srvs {
@@ -128,20 +112,13 @@ func TestServers_FindServer(t *testing.T) {
 	}
 
 	s2 := m.FindServer()
-	if s1.Equal(s2) {
+	must.NotEq(t, s1, s2)
-		t.Fatalf("Expected different server")
-	}
 }
 
 func TestServers_New(t *testing.T) {
 	ci.Parallel(t)
 
-	logger := testlog.HCLogger(t)
+	must.NotNil(t, servers.New(testlog.HCLogger(t), make(chan struct{}), &fauxConnPool{}))
-	shutdownCh := make(chan struct{})
-	m := servers.New(logger, shutdownCh, &fauxConnPool{})
-	if m == nil {
-		t.Fatalf("Manager nil")
-	}
 }
 
 func TestServers_NotifyFailedServer(t *testing.T) {
@@ -149,30 +126,22 @@ func TestServers_NotifyFailedServer(t *testing.T) {
 
 	m := testManager(t)
 
-	if m.NumServers() != 0 {
+	must.Zero(t, m.NumServers())
-		t.Fatalf("Expected zero servers to start")
-	}
 
 	s1 := &servers.Server{Addr: &fauxAddr{"s1"}}
 	s2 := &servers.Server{Addr: &fauxAddr{"s2"}}
 
 	// Try notifying for a server that is not managed by Manager
 	m.NotifyFailedServer(s1)
-	if m.NumServers() != 0 {
+	must.Zero(t, m.NumServers())
-		t.Fatalf("Expected zero servers to start")
-	}
 	m.SetServers([]*servers.Server{s1})
 
 	// Test again w/ a server not in the list
 	m.NotifyFailedServer(s2)
-	if m.NumServers() != 1 {
+	must.Eq(t, 1, m.NumServers())
-		t.Fatalf("Expected one server")
-	}
 
 	m.SetServers([]*servers.Server{s1, s2})
-	if m.NumServers() != 2 {
+	must.Eq(t, 2, m.NumServers())
-		t.Fatalf("Expected two servers")
-	}
 
 	// Grab a server
 	first := m.FindServer()
@@ -186,41 +155,28 @@ func TestServers_NotifyFailedServer(t *testing.T) {
 	// Fail the other server
 	m.NotifyFailedServer(second)
 	next := m.FindServer()
-	if !next.Equal(first) {
+	must.Equal(t, first, next)
-		t.Fatalf("Expected first server (still)")
-	}
 
 	// Fail the first
 	m.NotifyFailedServer(first)
 	next = m.FindServer()
-	if !next.Equal(second) {
+	must.True(t, next.Equal(second))
-		t.Fatalf("Expected second server")
-	}
 
 	// Fail the second
 	m.NotifyFailedServer(second)
 	next = m.FindServer()
-	if !next.Equal(first) {
+	must.True(t, next.Equal(first))
-		t.Fatalf("Expected first server")
-	}
 }
 
 func TestServers_NumServers(t *testing.T) {
 	ci.Parallel(t)
 
 	m := testManager(t)
-	var num int
+	must.Zero(t, m.NumServers())
-	num = m.NumServers()
-	if num != 0 {
-		t.Fatalf("Expected zero servers to start")
-	}
 
 	s := &servers.Server{Addr: &fauxAddr{"server1"}}
 	m.SetServers([]*servers.Server{s})
-	num = m.NumServers()
+	must.Eq(t, 1, m.NumServers())
-	if num != 1 {
-		t.Fatalf("Expected one server after SetServers")
-	}
 }
 
 func TestServers_RebalanceServers(t *testing.T) {
@@ -258,7 +214,7 @@ func TestServers_RebalanceServers(t *testing.T) {
 	// We have to allow for the fact that there won't always be a unique
 	// shuffle each pass, so we just look for smell here without the test
 	// being flaky.
-	if len(uniques) < int(maxServers*uniquePassRate) {
+	must.Greater(t, int(maxServers*uniquePassRate), len(uniques), must.Sprintf(
-		t.Fatalf("unique shuffle ratio too low: %d/%d", len(uniques), maxServers)
+		"unique shuffle ratio too low: %d/%d", len(uniques), maxServers),
-	}
+	)
 }