From a6baf7133a7c374a78058601e9c9839b63e4b18f Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 15 Jan 2018 14:56:38 -0800 Subject: [PATCH] Remove testing --- client/alloc_runner_test.go | 52 ++++++++++++++++++------------------ client/alloc_watcher_test.go | 2 +- client/client_test.go | 2 +- client/consul_testing.go | 12 +++------ client/gc_test.go | 42 ++++++++++++++--------------- client/task_runner_test.go | 2 +- client/testing.go | 2 +- 7 files changed, 55 insertions(+), 59 deletions(-) diff --git a/client/alloc_runner_test.go b/client/alloc_runner_test.go index 47348e29b..28abf0f43 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/command/agent/consul" + "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -62,8 +63,7 @@ func allocationBucketExists(tx *bolt.Tx, allocID string) bool { return alloc != nil } -func testAllocRunnerFromAlloc(alloc *structs.Allocation, restarts bool) (*MockAllocStateUpdater, *AllocRunner) { - logger := testLogger() +func testAllocRunnerFromAlloc(t *testing.T, alloc *structs.Allocation, restarts bool) (*MockAllocStateUpdater, *AllocRunner) { conf := config.DefaultConfig() conf.Node = mock.Node() conf.StateDir = os.TempDir() @@ -76,22 +76,22 @@ func testAllocRunnerFromAlloc(alloc *structs.Allocation, restarts bool) (*MockAl alloc.Job.Type = structs.JobTypeBatch } vclient := vaultclient.NewMockVaultClient() - ar := NewAllocRunner(logger, conf, db, upd.Update, alloc, vclient, newMockConsulServiceClient(), noopPrevAlloc{}) + ar := NewAllocRunner(testlog.Logger(t), conf, db, upd.Update, alloc, vclient, newMockConsulServiceClient(t), noopPrevAlloc{}) return upd, ar } -func testAllocRunner(restarts bool) (*MockAllocStateUpdater, *AllocRunner) { +func testAllocRunner(t *testing.T, restarts bool) (*MockAllocStateUpdater, *AllocRunner) { // Use mock driver alloc := mock.Alloc() task := alloc.Job.TaskGroups[0].Tasks[0] task.Driver = "mock_driver" task.Config["run_for"] = "500ms" - return testAllocRunnerFromAlloc(alloc, restarts) + return testAllocRunnerFromAlloc(t, alloc, restarts) } func TestAllocRunner_SimpleRun(t *testing.T) { t.Parallel() - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) go ar.Run() defer ar.Destroy() @@ -115,7 +115,7 @@ func TestAllocRunner_DeploymentHealth_Unhealthy_BadStart(t *testing.T) { assert := assert.New(t) // Ensure the task fails and restarts - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Make the task fail task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -163,7 +163,7 @@ func TestAllocRunner_DeploymentHealth_Unhealthy_Deadline(t *testing.T) { assert := assert.New(t) // Ensure the task fails and restarts - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Make the task block task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -211,7 +211,7 @@ func TestAllocRunner_DeploymentHealth_Healthy_NoChecks(t *testing.T) { t.Parallel() // Ensure the task fails and restarts - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Make the task run healthy task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -259,7 +259,7 @@ func TestAllocRunner_DeploymentHealth_Healthy_Checks(t *testing.T) { t.Parallel() // Ensure the task fails and restarts - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Make the task fail task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -352,7 +352,7 @@ func TestAllocRunner_DeploymentHealth_Unhealthy_Checks(t *testing.T) { assert := assert.New(t) // Ensure the task fails and restarts - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Make the task fail task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -421,7 +421,7 @@ func TestAllocRunner_DeploymentHealth_Healthy_UpdatedDeployment(t *testing.T) { t.Parallel() // Ensure the task fails and restarts - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Make the task run healthy task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -502,7 +502,7 @@ func TestAllocRunner_RetryArtifact(t *testing.T) { } alloc.Job.TaskGroups[0].Tasks = append(alloc.Job.TaskGroups[0].Tasks, badtask) - upd, ar := testAllocRunnerFromAlloc(alloc, true) + upd, ar := testAllocRunnerFromAlloc(t, alloc, true) go ar.Run() defer ar.Destroy() @@ -538,7 +538,7 @@ func TestAllocRunner_RetryArtifact(t *testing.T) { func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { t.Parallel() - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -637,7 +637,7 @@ func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { func TestAllocRunner_Destroy(t *testing.T) { t.Parallel() - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -693,7 +693,7 @@ func TestAllocRunner_Destroy(t *testing.T) { func TestAllocRunner_Update(t *testing.T) { t.Parallel() - _, ar := testAllocRunner(false) + _, ar := testAllocRunner(t, false) // Deep copy the alloc to avoid races when updating newAlloc := ar.Alloc().Copy() @@ -728,7 +728,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { "run_for": "10s", } - upd, ar := testAllocRunnerFromAlloc(alloc, false) + upd, ar := testAllocRunnerFromAlloc(t, alloc, false) go ar.Run() defer ar.Destroy() @@ -796,7 +796,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { func TestAllocRunner_SaveRestoreState_TerminalAlloc(t *testing.T) { t.Parallel() - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) ar.logger = prefixedTestLogger("ar1: ") // Ensure task takes some time @@ -929,7 +929,7 @@ func TestAllocRunner_SaveRestoreState_Upgrade(t *testing.T) { "run_for": "10s", } - upd, ar := testAllocRunnerFromAlloc(alloc, false) + upd, ar := testAllocRunnerFromAlloc(t, alloc, false) // Hack in old version to cause an upgrade on RestoreState origConfig := ar.config.Copy() ar.config.Version = &version.VersionInfo{Version: "0.5.6"} @@ -1112,7 +1112,7 @@ func TestAllocRunner_RestoreOldState(t *testing.T) { *alloc.Job.LookupTaskGroup(alloc.TaskGroup).RestartPolicy = structs.RestartPolicy{Attempts: 0} alloc.Job.Type = structs.JobTypeBatch vclient := vaultclient.NewMockVaultClient() - cclient := newMockConsulServiceClient() + cclient := newMockConsulServiceClient(t) ar := NewAllocRunner(logger, conf, db, upd.Update, alloc, vclient, cclient, noopPrevAlloc{}) defer ar.Destroy() @@ -1140,7 +1140,7 @@ func TestAllocRunner_RestoreOldState(t *testing.T) { func TestAllocRunner_TaskFailed_KillTG(t *testing.T) { t.Parallel() - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Create two tasks in the task group task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -1208,7 +1208,7 @@ func TestAllocRunner_TaskFailed_KillTG(t *testing.T) { func TestAllocRunner_TaskLeader_KillTG(t *testing.T) { t.Parallel() - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Create two tasks in the task group task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -1282,7 +1282,7 @@ func TestAllocRunner_TaskLeader_KillTG(t *testing.T) { // with a leader the leader is stopped before other tasks. func TestAllocRunner_TaskLeader_StopTG(t *testing.T) { t.Parallel() - upd, ar := testAllocRunner(false) + upd, ar := testAllocRunner(t, false) // Create 3 tasks in the task group task := ar.alloc.Job.TaskGroups[0].Tasks[0] @@ -1371,7 +1371,7 @@ func TestAllocRunner_TaskLeader_StopTG(t *testing.T) { // See https://github.com/hashicorp/nomad/issues/3420#issuecomment-341666932 func TestAllocRunner_TaskLeader_StopRestoredTG(t *testing.T) { t.Parallel() - _, ar := testAllocRunner(false) + _, ar := testAllocRunner(t, false) defer ar.Destroy() // Create a leader and follower task in the task group @@ -1468,7 +1468,7 @@ func TestAllocRunner_MoveAllocDir(t *testing.T) { task.Config = map[string]interface{}{ "run_for": "1s", } - upd, ar := testAllocRunnerFromAlloc(alloc, false) + upd, ar := testAllocRunnerFromAlloc(t, alloc, false) go ar.Run() defer ar.Destroy() @@ -1501,7 +1501,7 @@ func TestAllocRunner_MoveAllocDir(t *testing.T) { task.Config = map[string]interface{}{ "run_for": "1s", } - upd2, ar2 := testAllocRunnerFromAlloc(alloc2, false) + upd2, ar2 := testAllocRunnerFromAlloc(t, alloc2, false) // Set prevAlloc like Client does ar2.prevAlloc = newAllocWatcher(alloc2, ar, nil, ar2.config, ar2.logger, "") diff --git a/client/alloc_watcher_test.go b/client/alloc_watcher_test.go index d5f2c3f2a..43048363b 100644 --- a/client/alloc_watcher_test.go +++ b/client/alloc_watcher_test.go @@ -23,7 +23,7 @@ import ( // TestPrevAlloc_LocalPrevAlloc asserts that when a previous alloc runner is // set a localPrevAlloc will block on it. func TestPrevAlloc_LocalPrevAlloc(t *testing.T) { - _, prevAR := testAllocRunner(false) + _, prevAR := testAllocRunner(t, false) prevAR.alloc.Job.TaskGroups[0].Tasks[0].Config["run_for"] = "10s" newAlloc := mock.Alloc() diff --git a/client/client_test.go b/client/client_test.go index 98d42e9a1..d206ced8a 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -574,7 +574,7 @@ func TestClient_SaveRestoreState(t *testing.T) { // Create a new client logger := log.New(c1.config.LogOutput, "", log.LstdFlags) catalog := consul.NewMockCatalog(logger) - mockService := newMockConsulServiceClient() + mockService := newMockConsulServiceClient(t) mockService.logger = logger c2, err := NewClient(c1.config, catalog, mockService, logger) if err != nil { diff --git a/client/consul_testing.go b/client/consul_testing.go index 8703cdd21..4a2d2631b 100644 --- a/client/consul_testing.go +++ b/client/consul_testing.go @@ -2,16 +2,15 @@ package client import ( "fmt" - "io/ioutil" "log" - "os" "sync" - "testing" "github.com/hashicorp/nomad/client/driver" cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/command/agent/consul" + "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/structs" + "github.com/mitchellh/go-testing-interface" ) // mockConsulOp represents the register/deregister operations. @@ -49,13 +48,10 @@ type mockConsulServiceClient struct { allocRegistrationsFn func(allocID string) (*consul.AllocRegistration, error) } -func newMockConsulServiceClient() *mockConsulServiceClient { +func newMockConsulServiceClient(t testing.T) *mockConsulServiceClient { m := mockConsulServiceClient{ ops: make([]mockConsulOp, 0, 20), - logger: log.New(ioutil.Discard, "", 0), - } - if testing.Verbose() { - m.logger = log.New(os.Stderr, "", log.LstdFlags) + logger: testlog.Logger(t), } return &m } diff --git a/client/gc_test.go b/client/gc_test.go index 86f7ac221..f94b8bb7b 100644 --- a/client/gc_test.go +++ b/client/gc_test.go @@ -26,10 +26,10 @@ func TestIndexedGCAllocPQ(t *testing.T) { t.Parallel() pq := NewIndexedGCAllocPQ() - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) - _, ar2 := testAllocRunnerFromAlloc(mock.Alloc(), false) - _, ar3 := testAllocRunnerFromAlloc(mock.Alloc(), false) - _, ar4 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) + _, ar2 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) + _, ar3 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) + _, ar4 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) pq.Push(ar1) pq.Push(ar2) @@ -108,7 +108,7 @@ func TestAllocGarbageCollector_MarkForCollection(t *testing.T) { logger := testLogger() gc := NewAllocGarbageCollector(logger, &MockStatsCollector{}, &MockAllocCounter{}, gcConfig()) - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) gc.MarkForCollection(ar1) gcAlloc := gc.allocRunners.Pop() @@ -122,8 +122,8 @@ func TestAllocGarbageCollector_Collect(t *testing.T) { logger := testLogger() gc := NewAllocGarbageCollector(logger, &MockStatsCollector{}, &MockAllocCounter{}, gcConfig()) - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) - _, ar2 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) + _, ar2 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) gc.MarkForCollection(ar1) gc.MarkForCollection(ar2) @@ -143,8 +143,8 @@ func TestAllocGarbageCollector_CollectAll(t *testing.T) { logger := testLogger() gc := NewAllocGarbageCollector(logger, &MockStatsCollector{}, &MockAllocCounter{}, gcConfig()) - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) - _, ar2 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) + _, ar2 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) gc.MarkForCollection(ar1) gc.MarkForCollection(ar2) @@ -163,9 +163,9 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_EnoughSpace(t *testing.T) conf.ReservedDiskMB = 20 gc := NewAllocGarbageCollector(logger, statsCollector, &MockAllocCounter{}, conf) - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar1.waitCh) - _, ar2 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar2 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar2.waitCh) gc.MarkForCollection(ar1) gc.MarkForCollection(ar2) @@ -198,9 +198,9 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_GC_Partial(t *testing.T) { conf.ReservedDiskMB = 20 gc := NewAllocGarbageCollector(logger, statsCollector, &MockAllocCounter{}, conf) - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar1.waitCh) - _, ar2 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar2 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar2.waitCh) gc.MarkForCollection(ar1) gc.MarkForCollection(ar2) @@ -234,9 +234,9 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_GC_All(t *testing.T) { conf.ReservedDiskMB = 20 gc := NewAllocGarbageCollector(logger, statsCollector, &MockAllocCounter{}, conf) - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar1.waitCh) - _, ar2 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar2 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar2.waitCh) gc.MarkForCollection(ar1) gc.MarkForCollection(ar2) @@ -266,9 +266,9 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_GC_Fallback(t *testing.T) conf.ReservedDiskMB = 20 gc := NewAllocGarbageCollector(logger, statsCollector, &MockAllocCounter{}, conf) - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar1.waitCh) - _, ar2 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar2 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar2.waitCh) gc.MarkForCollection(ar1) gc.MarkForCollection(ar2) @@ -425,9 +425,9 @@ func TestAllocGarbageCollector_UsageBelowThreshold(t *testing.T) { conf.ReservedDiskMB = 20 gc := NewAllocGarbageCollector(logger, statsCollector, &MockAllocCounter{}, conf) - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar1.waitCh) - _, ar2 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar2 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar2.waitCh) gc.MarkForCollection(ar1) gc.MarkForCollection(ar2) @@ -457,9 +457,9 @@ func TestAllocGarbageCollector_UsedPercentThreshold(t *testing.T) { conf.ReservedDiskMB = 20 gc := NewAllocGarbageCollector(logger, statsCollector, &MockAllocCounter{}, conf) - _, ar1 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar1 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar1.waitCh) - _, ar2 := testAllocRunnerFromAlloc(mock.Alloc(), false) + _, ar2 := testAllocRunnerFromAlloc(t, mock.Alloc(), false) close(ar2.waitCh) gc.MarkForCollection(ar1) gc.MarkForCollection(ar2) diff --git a/client/task_runner_test.go b/client/task_runner_test.go index d91f5a96b..aa30f64b5 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -642,7 +642,7 @@ func TestTaskRunner_UnregisterConsul_Retries(t *testing.T) { ctx := testTaskRunnerFromAlloc(t, true, alloc) // Use mockConsulServiceClient - consul := newMockConsulServiceClient() + consul := newMockConsulServiceClient(t) ctx.tr.consul = consul ctx.tr.MarkReceived() diff --git a/client/testing.go b/client/testing.go index 9a88491b6..a86728365 100644 --- a/client/testing.go +++ b/client/testing.go @@ -33,7 +33,7 @@ func TestClient(t testing.T, cb func(c *config.Config)) *Client { logger := testlog.Logger(t) catalog := consul.NewMockCatalog(logger) - mockService := newMockConsulServiceClient() + mockService := newMockConsulServiceClient(t) mockService.logger = logger client, err := NewClient(conf, catalog, mockService, logger) if err != nil {