From aa62781aa7cc491cdcc1ebeba13734aa2d45b899 Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Thu, 16 Jan 2020 17:24:36 +0800 Subject: [PATCH 1/3] unit-test: reconstuct TestMain os.Exit will skip all deferred instructions. So we should reconstruct TestMain to leave all setup-related code in setup(), and all cleanup-related code in shutdown(). Fixes: #2398 Signed-off-by: Penny Zheng --- virtcontainers/store/manager_test.go | 32 +++++++++++++++++----------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/virtcontainers/store/manager_test.go b/virtcontainers/store/manager_test.go index 713cb30bd3..be6e56a3e6 100644 --- a/virtcontainers/store/manager_test.go +++ b/virtcontainers/store/manager_test.go @@ -27,6 +27,9 @@ var storeRoot, storeRootDir = func() (string, string) { dir, _ := ioutil.TempDir("", "") return "file://" + dir, dir }() +var testDir = "" +var ConfigStoragePathSaved = func() string { return "" } +var RunStoragePathSaved = func() string { return "" } func TestNewStore(t *testing.T) { s, err := New(context.Background(), storeRoot) @@ -111,22 +114,31 @@ func TestManagerFindStore(t *testing.T) { // TestMain is the common main function used by ALL the test functions // for the store. func TestMain(m *testing.M) { - testDir, err := ioutil.TempDir("", "store-tmp-") + setup() + rt := m.Run() + shutdown() + os.Exit(rt) +} + +func shutdown() { + os.RemoveAll(testDir) + ConfigStoragePath = ConfigStoragePathSaved + RunStoragePath = RunStoragePathSaved +} + +func setup() { + var err error + testDir, err = ioutil.TempDir("", "store-tmp-") if err != nil { panic(err) } - ConfigStoragePathSaved := ConfigStoragePath - RunStoragePathSaved := RunStoragePath + ConfigStoragePathSaved = ConfigStoragePath + RunStoragePathSaved = RunStoragePath // allow the tests to run without affecting the host system. ConfigStoragePath = func() string { return filepath.Join(testDir, StoragePathSuffix, "config") } RunStoragePath = func() string { return filepath.Join(testDir, StoragePathSuffix, "run") } - defer func() { - ConfigStoragePath = ConfigStoragePathSaved - RunStoragePath = RunStoragePathSaved - }() - // set now that ConfigStoragePath has been overridden. sandboxDirConfig = filepath.Join(ConfigStoragePath(), testSandboxID) sandboxFileConfig = filepath.Join(ConfigStoragePath(), testSandboxID, ConfigurationFile) @@ -134,8 +146,4 @@ func TestMain(m *testing.M) { sandboxDirLock = filepath.Join(RunStoragePath(), testSandboxID) sandboxFileState = filepath.Join(RunStoragePath(), testSandboxID, StateFile) sandboxFileLock = filepath.Join(RunStoragePath(), testSandboxID, LockFile) - - ret := m.Run() - - os.Exit(ret) } From 0244d95edd743bf149ff21a80e203f370ccb2156 Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Thu, 16 Jan 2020 17:49:13 +0800 Subject: [PATCH 2/3] unit-test: delete what ioutil.TempDir() creates Normally, ioutil.TempDir will create a new temporary dir under /tmp. And we should do cleaning up after ioutil.TempDir(). Fixes: #2398 Signed-off-by: Penny Zheng --- virtcontainers/kata_agent_test.go | 8 ++++++-- virtcontainers/vm_test.go | 13 ++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index 79a94c8dac..dcd6db82c2 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -636,6 +636,7 @@ func TestAgentConfigure(t *testing.T) { dir, err := ioutil.TempDir("", "kata-agent-test") assert.Nil(err) + defer os.RemoveAll(dir) k := &kataAgent{} h := &mockHypervisor{} @@ -758,6 +759,7 @@ func TestAgentCreateContainer(t *testing.T) { dir, err := ioutil.TempDir("", "kata-agent-test") assert.Nil(err) + defer os.RemoveAll(dir) err = k.configure(&mockHypervisor{}, sandbox.id, dir, true, KataAgentConfig{}) assert.Nil(err) @@ -904,8 +906,10 @@ func TestKataCleanupSandbox(t *testing.T) { s := Sandbox{ id: "testFoo", } - dir := path.Join(kataHostSharedDir(), s.id) - err := os.MkdirAll(dir, 0777) + + dir := kataHostSharedDir() + defer os.RemoveAll(dir) + err := os.MkdirAll(path.Join(dir, s.id), 0777) assert.Nil(err) k := &kataAgent{ctx: context.Background()} diff --git a/virtcontainers/vm_test.go b/virtcontainers/vm_test.go index c9052460ab..e449590f57 100644 --- a/virtcontainers/vm_test.go +++ b/virtcontainers/vm_test.go @@ -8,6 +8,7 @@ package virtcontainers import ( "context" "io/ioutil" + "os" "testing" "github.com/kata-containers/runtime/virtcontainers/utils" @@ -17,7 +18,10 @@ import ( func TestNewVM(t *testing.T) { assert := assert.New(t) - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") + testDir, err := ioutil.TempDir("", "vmfactory-tmp-") + assert.Nil(err) + defer os.RemoveAll(testDir) + config := VMConfig{ HypervisorType: MockHypervisor, AgentType: NoopAgentType, @@ -31,7 +35,7 @@ func TestNewVM(t *testing.T) { ctx := context.Background() var vm *VM - _, err := NewVM(ctx, config) + _, err = NewVM(ctx, config) assert.Error(err) config.HypervisorConfig = hyperConfig @@ -82,7 +86,10 @@ func TestVMConfigValid(t *testing.T) { err := config.Valid() assert.Error(err) - testDir, _ := ioutil.TempDir("", "vmfactory-tmp-") + testDir, err := ioutil.TempDir("", "vmfactory-tmp-") + assert.Nil(err) + defer os.RemoveAll(testDir) + config.HypervisorConfig = HypervisorConfig{ KernelPath: testDir, InitrdPath: testDir, From 7186c01d6e8c0ebaf75db2231613c4ee761e7c4b Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Fri, 17 Jan 2020 15:29:50 +0800 Subject: [PATCH 3/3] unit-test: delete what ioutil.TempFile creates ioutil.TempFile creates a new temporary file in the directory dir. It is the caller's responsibility to remove the file when no longer needed. Fixes: #2398 Signed-off-by: Penny Zheng --- virtcontainers/acrn_arch_base_test.go | 2 ++ virtcontainers/qemu_arch_base_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/virtcontainers/acrn_arch_base_test.go b/virtcontainers/acrn_arch_base_test.go index 69fa55093f..9c9c887596 100644 --- a/virtcontainers/acrn_arch_base_test.go +++ b/virtcontainers/acrn_arch_base_test.go @@ -9,6 +9,7 @@ import ( "fmt" "io/ioutil" "net" + "os" "path/filepath" "testing" @@ -128,6 +129,7 @@ func TestAcrnArchBaseAppendImage(t *testing.T) { image, err := ioutil.TempFile("", "img") assert.NoError(err) + defer os.Remove(image.Name()) err = image.Close() assert.NoError(err) diff --git a/virtcontainers/qemu_arch_base_test.go b/virtcontainers/qemu_arch_base_test.go index 5a0432eff5..7a91363b3b 100644 --- a/virtcontainers/qemu_arch_base_test.go +++ b/virtcontainers/qemu_arch_base_test.go @@ -9,6 +9,7 @@ import ( "fmt" "io/ioutil" "net" + "os" "path/filepath" "testing" @@ -287,6 +288,7 @@ func TestQemuArchBaseAppendImage(t *testing.T) { image, err := ioutil.TempFile("", "img") assert.NoError(err) + defer os.Remove(image.Name()) err = image.Close() assert.NoError(err)