From 4e8e0de8f0593495df7f8ce49ab6929034a8bdfc Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 13 Aug 2020 17:03:55 -0400 Subject: [PATCH 1/6] testing: remove unused fields from TestACLAgent --- agent/acl_test.go | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/agent/acl_test.go b/agent/acl_test.go index 3d25b965a8..e22649db69 100644 --- a/agent/acl_test.go +++ b/agent/acl_test.go @@ -3,6 +3,7 @@ package agent import ( "fmt" "io" + "os" "testing" "time" @@ -25,23 +26,6 @@ type authzResolver func(string) (structs.ACLIdentity, acl.Authorizer, error) type identResolver func(string) (structs.ACLIdentity, error) type TestACLAgent struct { - // Name is an optional name of the agent. - Name string - - HCL string - - // Config is the agent configuration. If Config is nil then - // TestConfig() is used. If Config.DataDir is set then it is - // the callers responsibility to clean up the data directory. - // Otherwise, a temporary data directory is created and removed - // when Shutdown() is called. - Config *config.RuntimeConfig - - // DataDir is the data directory which is used when Config.DataDir - // is not set. It is created automatically and removed when - // Shutdown() is called. - DataDir string - resolveAuthzFn authzResolver resolveIdentFn identResolver @@ -52,11 +36,15 @@ type TestACLAgent struct { // Basically it needs a local state for some of the vet* functions, a logger and a delegate. // The key is that we are the delegate so we can control the ResolveToken responses func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzResolver, resolveIdent identResolver) *TestACLAgent { - a := &TestACLAgent{Name: name, HCL: hcl, resolveAuthzFn: resolveAuthz, resolveIdentFn: resolveIdent} - dataDir := `data_dir = "acl-agent"` + a := &TestACLAgent{resolveAuthzFn: resolveAuthz, resolveIdentFn: resolveIdent} + + dataDir := testutil.TempDir(t, "acl-agent") + t.Cleanup(func() { + os.RemoveAll(dataDir) + }) logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{ - Name: a.Name, + Name: name, Level: hclog.Debug, Output: testutil.NewLogBuffer(t), }) @@ -66,22 +54,22 @@ func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzRe WithBuilderOpts(config.BuilderOpts{ HCL: []string{ TestConfigHCL(NodeID()), - a.HCL, - dataDir, + hcl, + fmt.Sprintf(`data_dir = "%s"`, dataDir), }, }), } agent, err := New(opts...) require.NoError(t, err) - a.Config = agent.GetConfig() + cfg := agent.GetConfig() a.Agent = agent agent.logger = logger agent.MemSink = metrics.NewInmemSink(1*time.Second, time.Minute) a.Agent.delegate = a - a.Agent.State = local.NewState(LocalConfig(a.Config), a.Agent.logger, a.Agent.tokens) + a.Agent.State = local.NewState(LocalConfig(cfg), logger, a.Agent.tokens) a.Agent.State.TriggerSyncChanges = func() {} return a } From b1679508d446e1af185c3456d089db0aaaded7c8 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 13 Aug 2020 17:09:37 -0400 Subject: [PATCH 2/6] testing: use t.Cleanup in TestAgent for returnPorts --- agent/testagent.go | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/agent/testagent.go b/agent/testagent.go index 387c3efbea..74570a368e 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -58,12 +58,9 @@ type TestAgent struct { // the callers responsibility to clean up the data directory. // Otherwise, a temporary data directory is created and removed // when Shutdown() is called. + // TODO: Config *config.RuntimeConfig - // returnPortsFn will put the ports claimed for the test back into the - // general freeport pool - returnPortsFn func() - // LogOutput is the sink for the logs. If nil, logs are written // to os.Stderr. LogOutput io.Writer @@ -71,9 +68,11 @@ type TestAgent struct { // DataDir is the data directory which is used when Config.DataDir // is not set. It is created automatically and removed when // Shutdown() is called. + // TODO: DataDir string // Key is the optional encryption key for the LAN and WAN keyring. + // TODO: Key string // UseTLS, if true, will disable the HTTP port and enable the HTTPS @@ -198,7 +197,7 @@ func (a *TestAgent) Start(t *testing.T) (err error) { }) portsConfig, returnPortsFn := randomPortsSource(a.UseTLS) - a.returnPortsFn = returnPortsFn + t.Cleanup(returnPortsFn) nodeID := NodeID() @@ -221,13 +220,6 @@ func (a *TestAgent) Start(t *testing.T) (err error) { ), } - defer func() { - if err != nil && a.returnPortsFn != nil { - a.returnPortsFn() - a.returnPortsFn = nil - } - }() - // write the keyring if a.Key != "" { writeKey := func(key, filename string) error { @@ -357,14 +349,6 @@ func (a *TestAgent) Shutdown() error { return nil } - // Return ports last of all - defer func() { - if a.returnPortsFn != nil { - a.returnPortsFn() - a.returnPortsFn = nil - } - }() - // shutdown agent before endpoints defer a.Agent.ShutdownEndpoints() if err := a.Agent.ShutdownAgent(); err != nil { From 3a4e62836ba2930e7d6419ed9d56e8d738c623f9 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 13 Aug 2020 17:17:21 -0400 Subject: [PATCH 3/6] testing: Remove TestAgent.Key and change TestAgent.DataDir TestAgent.Key was only used by 3 tests. Extracting it from the common helper that is used in hundreds of tests helps keep the shared part small and more focused. This required a second change (which I was planning on making anyway), which was to change the behaviour of DataDir. Now in all cases the TestAgent will use the DataDir, and clean it up once the test is complete. --- agent/agent_test.go | 39 +++++-------------- agent/keyring_test.go | 30 +++++++++++++-- agent/service_manager_test.go | 17 ++------- agent/testagent.go | 71 +++++------------------------------ 4 files changed, 50 insertions(+), 107 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index 4b109b9675..4746fd3ff9 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1536,15 +1536,12 @@ func TestAgent_RestoreServiceWithAliasCheck(t *testing.T) { a.logger.Info("testharness: " + fmt.Sprintf(format, args...)) } - dataDir := testutil.TempDir(t, "agent") // we manage the data dir cfg := ` server = false bootstrap = false enable_central_service_config = false - data_dir = "` + dataDir + `" ` - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) - defer os.RemoveAll(dataDir) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() testCtx, testCancel := context.WithCancel(context.Background()) @@ -1627,7 +1624,7 @@ node_name = "` + a.Config.NodeName + `" t.Helper() // Reload and retain former NodeID and data directory. - a2 := StartTestAgent(t, TestAgent{HCL: futureHCL, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{HCL: futureHCL, DataDir: a.DataDir}) defer a2.Shutdown() a = nil @@ -2012,15 +2009,11 @@ func TestAgent_PersistService(t *testing.T) { func testAgent_PersistService(t *testing.T, extraHCL string) { t.Helper() - dataDir := testutil.TempDir(t, "agent") // we manage the data dir - defer os.RemoveAll(dataDir) - cfg := ` server = false bootstrap = false - data_dir = "` + dataDir + `" ` + extraHCL - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() svc := &structs.NodeService{ @@ -2086,7 +2079,7 @@ func testAgent_PersistService(t *testing.T, extraHCL string) { a.Shutdown() // Should load it back during later start - a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: a.DataDir}) defer a2.Shutdown() restored := a2.State.ServiceState(structs.NewServiceID(svc.ID, nil)) @@ -2224,15 +2217,11 @@ func TestAgent_PurgeServiceOnDuplicate(t *testing.T) { func testAgent_PurgeServiceOnDuplicate(t *testing.T, extraHCL string) { t.Helper() - dataDir := testutil.TempDir(t, "agent") // we manage the data dir - defer os.RemoveAll(dataDir) - cfg := ` - data_dir = "` + dataDir + `" server = false bootstrap = false ` + extraHCL - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() svc1 := &structs.NodeService{ @@ -2255,7 +2244,7 @@ func testAgent_PurgeServiceOnDuplicate(t *testing.T, extraHCL string) { tags = ["bar"] port = 9000 } - `, DataDir: dataDir}) + `, DataDir: a.DataDir}) defer a2.Shutdown() sid := svc1.CompoundServiceID() @@ -2269,15 +2258,12 @@ func testAgent_PurgeServiceOnDuplicate(t *testing.T, extraHCL string) { func TestAgent_PersistCheck(t *testing.T) { t.Parallel() - dataDir := testutil.TempDir(t, "agent") // we manage the data dir cfg := ` - data_dir = "` + dataDir + `" server = false bootstrap = false enable_script_checks = true ` - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) - defer os.RemoveAll(dataDir) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() check := &structs.HealthCheck{ @@ -2333,7 +2319,7 @@ func TestAgent_PersistCheck(t *testing.T) { a.Shutdown() // Should load it back during later start - a2 := StartTestAgent(t, TestAgent{Name: "Agent2", HCL: cfg, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{Name: "Agent2", HCL: cfg, DataDir: a.DataDir}) defer a2.Shutdown() result := requireCheckExists(t, a2, check.CheckID) @@ -2384,18 +2370,14 @@ func TestAgent_PurgeCheck(t *testing.T) { func TestAgent_PurgeCheckOnDuplicate(t *testing.T) { t.Parallel() nodeID := NodeID() - dataDir := testutil.TempDir(t, "agent") a := StartTestAgent(t, TestAgent{ - DataDir: dataDir, HCL: ` node_id = "` + nodeID + `" node_name = "Node ` + nodeID + `" - data_dir = "` + dataDir + `" server = false bootstrap = false enable_script_checks = true `}) - defer os.RemoveAll(dataDir) defer a.Shutdown() check1 := &structs.HealthCheck{ @@ -2415,11 +2397,10 @@ func TestAgent_PurgeCheckOnDuplicate(t *testing.T) { // Start again with the check registered in config a2 := StartTestAgent(t, TestAgent{ Name: "Agent2", - DataDir: dataDir, + DataDir: a.DataDir, HCL: ` node_id = "` + nodeID + `" node_name = "Node ` + nodeID + `" - data_dir = "` + dataDir + `" server = false bootstrap = false enable_script_checks = true @@ -2434,7 +2415,7 @@ func TestAgent_PurgeCheckOnDuplicate(t *testing.T) { defer a2.Shutdown() cid := check1.CompoundCheckID() - file := filepath.Join(dataDir, checksDir, cid.StringHash()) + file := filepath.Join(a.DataDir, checksDir, cid.StringHash()) if _, err := os.Stat(file); err == nil { t.Fatalf("should have removed persisted check") } diff --git a/agent/keyring_test.go b/agent/keyring_test.go index 2b160d4f78..16acc01611 100644 --- a/agent/keyring_test.go +++ b/agent/keyring_test.go @@ -54,7 +54,10 @@ func TestAgent_LoadKeyrings(t *testing.T) { // Server should auto-load LAN and WAN keyring files t.Run("server with keys", func(t *testing.T) { - a2 := StartTestAgent(t, TestAgent{Key: key}) + dataDir := testutil.TempDir(t, "keyfile") + writeKeyRings(t, key, dataDir) + + a2 := StartTestAgent(t, TestAgent{DataDir: dataDir}) defer a2.Shutdown() c2 := a2.consulConfig() @@ -80,10 +83,16 @@ func TestAgent_LoadKeyrings(t *testing.T) { // Client should auto-load only the LAN keyring file t.Run("client with keys", func(t *testing.T) { - a3 := StartTestAgent(t, TestAgent{HCL: ` + dataDir := testutil.TempDir(t, "keyfile") + writeKeyRings(t, key, dataDir) + + a3 := StartTestAgent(t, TestAgent{ + HCL: ` server = false bootstrap = false - `, Key: key}) + `, + DataDir: dataDir, + }) defer a3.Shutdown() c3 := a3.consulConfig() @@ -105,6 +114,16 @@ func TestAgent_LoadKeyrings(t *testing.T) { }) } +func writeKeyRings(t *testing.T, key string, dataDir string) { + t.Helper() + writeKey := func(key, filename string) { + path := filepath.Join(dataDir, filename) + require.NoError(t, initKeyring(path, key), "Error creating keyring %s", path) + } + writeKey(key, SerfLANKeyring) + writeKey(key, SerfWANKeyring) +} + func TestAgent_InmemKeyrings(t *testing.T) { t.Parallel() key := "tbLJg26ZJyJ9pK3qhc9jig==" @@ -272,11 +291,14 @@ func TestAgentKeyring_ACL(t *testing.T) { key1 := "tbLJg26ZJyJ9pK3qhc9jig==" key2 := "4leC33rgtXKIVUr9Nr0snQ==" + dataDir := testutil.TempDir(t, "keyfile") + writeKeyRings(t, key1, dataDir) + a := StartTestAgent(t, TestAgent{HCL: TestACLConfig() + ` acl_datacenter = "dc1" acl_master_token = "root" acl_default_policy = "deny" - `, Key: key1}) + `, DataDir: dataDir}) defer a.Shutdown() // List keys without access fails diff --git a/agent/service_manager_test.go b/agent/service_manager_test.go index e71e21f222..8cb3ce2a24 100644 --- a/agent/service_manager_test.go +++ b/agent/service_manager_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" "github.com/stretchr/testify/require" @@ -293,16 +292,12 @@ func TestServiceManager_PersistService_API(t *testing.T) { ) // Now launch a single client agent - dataDir := testutil.TempDir(t, "agent") // we manage the data dir - defer os.RemoveAll(dataDir) - cfg := ` enable_central_service_config = true server = false bootstrap = false - data_dir = "` + dataDir + `" ` - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() // Join first @@ -465,7 +460,7 @@ func TestServiceManager_PersistService_API(t *testing.T) { serverAgent.Shutdown() // Should load it back during later start. - a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: a.DataDir}) defer a2.Shutdown() { @@ -510,9 +505,6 @@ func TestServiceManager_PersistService_ConfigFiles(t *testing.T) { ) // Now launch a single client agent - dataDir := testutil.TempDir(t, "agent") // we manage the data dir - defer os.RemoveAll(dataDir) - serviceSnippet := ` service = { kind = "connect-proxy" @@ -535,12 +527,11 @@ func TestServiceManager_PersistService_ConfigFiles(t *testing.T) { cfg := ` enable_central_service_config = true - data_dir = "` + dataDir + `" server = false bootstrap = false ` + serviceSnippet - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() // Join first @@ -639,7 +630,7 @@ func TestServiceManager_PersistService_ConfigFiles(t *testing.T) { serverAgent.Shutdown() // Should load it back during later start. - a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: a.DataDir}) defer a2.Shutdown() { diff --git a/agent/testagent.go b/agent/testagent.go index 74570a368e..426de1a917 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -8,13 +8,11 @@ import ( "crypto/x509" "fmt" "io" - "io/ioutil" "math/rand" "net/http/httptest" "os" "path/filepath" "strconv" - "strings" "testing" "text/template" "time" @@ -58,23 +56,18 @@ type TestAgent struct { // the callers responsibility to clean up the data directory. // Otherwise, a temporary data directory is created and removed // when Shutdown() is called. - // TODO: Config *config.RuntimeConfig // LogOutput is the sink for the logs. If nil, logs are written // to os.Stderr. LogOutput io.Writer - // DataDir is the data directory which is used when Config.DataDir - // is not set. It is created automatically and removed when - // Shutdown() is called. - // TODO: + // DataDir may be set to a directory which exists. If is it not set, + // TestAgent.Start will create one and set DataDir to the directory path. + // In all cases the agent will be configured to use this path as the data directory, + // and the directory will be removed once the test ends. DataDir string - // Key is the optional encryption key for the LAN and WAN keyring. - // TODO: - Key string - // UseTLS, if true, will disable the HTTP port and enable the HTTPS // one. UseTLS bool @@ -154,35 +147,14 @@ func (a *TestAgent) Start(t *testing.T) (err error) { name = "TestAgent" } - var cleanupTmpDir = func() { - // Clean out the data dir if we are responsible for it before we - // try again, since the old ports may have gotten written to - // the data dir, such as in the Raft configuration. - if a.DataDir != "" { - if err := os.RemoveAll(a.DataDir); err != nil { - fmt.Printf("%s Error resetting data dir: %s", name, err) - } - } - } - - var hclDataDir string if a.DataDir == "" { - dirname := "agent" - if name != "" { - dirname = name + "-agent" - } - dirname = strings.Replace(dirname, "/", "_", -1) - d, err := ioutil.TempDir(TempDir, dirname) - if err != nil { - return fmt.Errorf("Error creating data dir %s: %s", filepath.Join(TempDir, dirname), err) - } - // Convert windows style path to posix style path - // to avoid illegal char escape error when hcl - // parsing. - d = filepath.ToSlash(d) - hclDataDir = `data_dir = "` + d + `"` - a.DataDir = d + dirname := name + "-agent" + a.DataDir = testutil.TempDir(t, dirname) } + // Convert windows style path to posix style path to avoid illegal char escape + // error when hcl parsing. + d := filepath.ToSlash(a.DataDir) + hclDataDir := fmt.Sprintf(`data_dir = "%s"`, d) logOutput := a.LogOutput if logOutput == nil { @@ -220,29 +192,8 @@ func (a *TestAgent) Start(t *testing.T) (err error) { ), } - // write the keyring - if a.Key != "" { - writeKey := func(key, filename string) error { - path := filepath.Join(a.DataDir, filename) - if err := initKeyring(path, key); err != nil { - cleanupTmpDir() - return fmt.Errorf("Error creating keyring %s: %s", path, err) - } - return nil - } - if err = writeKey(a.Key, SerfLANKeyring); err != nil { - cleanupTmpDir() - return err - } - if err = writeKey(a.Key, SerfWANKeyring); err != nil { - cleanupTmpDir() - return err - } - } - agent, err := New(opts...) if err != nil { - cleanupTmpDir() return fmt.Errorf("Error creating agent: %s", err) } @@ -253,7 +204,6 @@ func (a *TestAgent) Start(t *testing.T) (err error) { id := string(a.Config.NodeID) if err := agent.Start(context.Background()); err != nil { - cleanupTmpDir() agent.ShutdownAgent() agent.ShutdownEndpoints() @@ -266,7 +216,6 @@ func (a *TestAgent) Start(t *testing.T) (err error) { a.Agent.StartSync() if err := a.waitForUp(); err != nil { - cleanupTmpDir() a.Shutdown() t.Logf("Error while waiting for test agent to start: %v", err) return errwrap.Wrapf(name+": {{err}}", err) From 1912c5ad89ec7ff0035d0038aa4002a06ca0182e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 13 Aug 2020 17:48:46 -0400 Subject: [PATCH 4/6] testing: wait until monitor has started before shutdown This commit fixes a test that I saw flake locally while running tests. The test output from the monitor started immediately after the line the test was looking for. To fix the problem a channel is closed when the goroutine starts. Shutdown is not called until this channel is closed, which seems to greatly reduce the chance of a flake. --- agent/agent_endpoint_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index d7d8f94b41..6f2b9f7bfe 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -4542,12 +4542,15 @@ func TestAgent_Monitor(t *testing.T) { req = req.WithContext(cancelCtx) resp := httptest.NewRecorder() - errCh := make(chan error) + chErr := make(chan error) + chStarted := make(chan struct{}) go func() { + close(chStarted) _, err := a.srv.AgentMonitor(resp, req) - errCh <- err + chErr <- err }() + <-chStarted require.NoError(t, a.Shutdown()) // Wait until we have received some type of logging output @@ -4556,7 +4559,7 @@ func TestAgent_Monitor(t *testing.T) { }, 3*time.Second, 100*time.Millisecond) cancelFunc() - err := <-errCh + err := <-chErr require.NoError(t, err) got := resp.Body.String() From 2b920ad19999e23f1c907a0371a4eeafb1031b37 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 13 Aug 2020 18:03:04 -0400 Subject: [PATCH 5/6] testing: fix flaky test TestDNS_NonExistentDC_RPC I saw this test flake locally, and it was easy to reproduce with -count=10. The failure was: 'TestAgent.dns: rpc error: error=No known Consul servers'. Waiting for the agent seems to fix it. --- agent/dns_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/agent/dns_test.go b/agent/dns_test.go index a5aa787c4b..07c4211871 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -5829,16 +5829,12 @@ func TestDNS_NonExistentDC_RPC(t *testing.T) { server = false `) defer c.Shutdown() - testrpc.WaitForLeader(t, s.RPC, "dc1") // Join LAN cluster addr := fmt.Sprintf("127.0.0.1:%d", s.Config.SerfPortLAN) _, err := c.JoinLAN([]string{addr}) require.NoError(t, err) - retry.Run(t, func(r *retry.R) { - require.Len(r, s.LANMembers(), 2) - require.Len(r, c.LANMembers(), 2) - }) + testrpc.WaitForTestAgent(t, c.RPC, "dc1") m := new(dns.Msg) m.SetQuestion("consul.service.dc2.consul.", dns.TypeANY) From 070e843113a8fe488ed7e41700c6233e4da96513 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 14 Aug 2020 13:19:06 -0400 Subject: [PATCH 6/6] testutil: Add t.Cleanup to TempDir TempDir registers a Cleanup so that the directory is always removed. To disable to cleanup, set the TEST_NOCLEANUP env var. --- agent/testagent.go | 4 ---- sdk/testutil/io.go | 17 ++++++++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/agent/testagent.go b/agent/testagent.go index 426de1a917..a946b32c46 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -10,7 +10,6 @@ import ( "io" "math/rand" "net/http/httptest" - "os" "path/filepath" "strconv" "testing" @@ -38,9 +37,6 @@ func init() { rand.Seed(time.Now().UnixNano()) // seed random number generator } -// TempDir defines the base dir for temporary directories. -var TempDir = os.TempDir() - // TestAgent encapsulates an Agent with a default configuration and // startup procedure suitable for testing. It panics if there are errors // during creation or startup instead of returning errors. It manages a diff --git a/sdk/testutil/io.go b/sdk/testutil/io.go index a137fc6a3f..28f215b677 100644 --- a/sdk/testutil/io.go +++ b/sdk/testutil/io.go @@ -29,21 +29,28 @@ func init() { } } +var noCleanup = strings.ToLower(os.Getenv("TEST_NOCLEANUP")) == "true" + // TempDir creates a temporary directory within tmpdir // with the name 'testname-name'. If the directory cannot // be created t.Fatal is called. func TempDir(t *testing.T, name string) string { - if t != nil && t.Name() != "" { - name = t.Name() + "-" + name + if t == nil { + panic("argument t must be non-nil") } + name = t.Name() + "-" + name name = strings.Replace(name, "/", "_", -1) d, err := ioutil.TempDir(tmpdir, name) if err != nil { - if t == nil { - panic(err) - } t.Fatalf("err: %s", err) } + t.Cleanup(func() { + if noCleanup { + t.Logf("skipping cleanup because TEST_NOCLEANUP was enabled") + return + } + os.RemoveAll(d) + }) return d }