From d6d38dae13f1c93744537b349a91a74e8f8e579e Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Fri, 13 Jul 2018 17:44:23 +0100 Subject: [PATCH 1/3] cli: update_test: defer remove tmpfile Ensure we remove the tmpfile used for testing. Signed-off-by: Graham Whaley --- cli/update_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/update_test.go b/cli/update_test.go index e2c2e38ce..713382f8b 100644 --- a/cli/update_test.go +++ b/cli/update_test.go @@ -111,6 +111,7 @@ func TestUpdateCLIFailure(t *testing.T) { // json decode error f, err := ioutil.TempFile("", "resources") + defer os.Remove(f.Name()) assert.NoError(err) assert.NotNil(f) f.WriteString("no json") From 73c8286c7eceb17ec1e5dc7efd8addb8f484c491 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Wed, 18 Jul 2018 10:55:30 +0100 Subject: [PATCH 2/3] cli: tests: remove the tmpdir to the config.json We were defer removing the temporary config.json files but not the tmpdir path we had created to store them in. Expose that path out so we can defer removeall it. Fixes: #480 Signed-off-by: Graham Whaley --- cli/delete_test.go | 24 +++++++++++++++--------- cli/exec_test.go | 24 ++++++++++++++++-------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/cli/delete_test.go b/cli/delete_test.go index 20fea73c8..2a927bcc4 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -121,7 +121,7 @@ func TestDeleteInvalidConfig(t *testing.T) { assert.False(vcmock.IsMockError(err)) } -func testConfigSetup(t *testing.T) string { +func testConfigSetup(t *testing.T) (rootPath string, configPath string) { assert := assert.New(t) tmpdir, err := ioutil.TempDir("", "") @@ -135,8 +135,8 @@ func testConfigSetup(t *testing.T) string { assert.NoError(err) // config json path - configPath := filepath.Join(bundlePath, "config.json") - return configPath + configPath = filepath.Join(bundlePath, "config.json") + return tmpdir, configPath } func TestDeleteSandbox(t *testing.T) { @@ -146,7 +146,8 @@ func TestDeleteSandbox(t *testing.T) { MockID: testSandboxID, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -223,7 +224,8 @@ func TestDeleteInvalidContainerType(t *testing.T) { MockID: testSandboxID, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -261,7 +263,8 @@ func TestDeleteSandboxRunning(t *testing.T) { MockID: testSandboxID, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -340,7 +343,8 @@ func TestDeleteRunningContainer(t *testing.T) { }, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -422,7 +426,8 @@ func TestDeleteContainer(t *testing.T) { }, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -523,7 +528,8 @@ func TestDeleteCLIFunctionSuccess(t *testing.T) { }, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) diff --git a/cli/exec_test.go b/cli/exec_test.go index d45bd3091..3fdf8b8f0 100644 --- a/cli/exec_test.go +++ b/cli/exec_test.go @@ -89,7 +89,8 @@ func TestExecuteErrors(t *testing.T) { assert.False(vcmock.IsMockError(err)) // Container state undefined - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -147,7 +148,8 @@ func TestExecuteErrorReadingProcessJson(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -195,7 +197,8 @@ func TestExecuteErrorOpeningConsole(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -261,7 +264,8 @@ func TestExecuteWithFlags(t *testing.T) { flagSet.Parse([]string{testContainerID, "/tmp/foo"}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -349,7 +353,8 @@ func TestExecuteWithFlagsDetached(t *testing.T) { flagSet.Parse([]string{testContainerID, "/tmp/foo"}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -427,7 +432,8 @@ func TestExecuteWithInvalidProcessJson(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -478,7 +484,8 @@ func TestExecuteWithValidProcessJson(t *testing.T) { flagSet.Parse([]string{testContainerID, "/tmp/foo"}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -578,7 +585,8 @@ func TestExecuteWithEmptyEnvironmentValue(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) From 50b445cf35b31e2c0ea20f570fdd086e7d0fa77c Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Wed, 18 Jul 2018 11:05:02 +0100 Subject: [PATCH 3/3] cli: tests: Clarify who cleans up tmpdir Add a comment to clarify that the caller of testRunContainerSetup() cleans up the tmpdir. Signed-off-by: Graham Whaley --- cli/run_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/run_test.go b/cli/run_test.go index 9377a4900..2d051b534 100644 --- a/cli/run_test.go +++ b/cli/run_test.go @@ -168,6 +168,7 @@ func testRunContainerSetup(t *testing.T) runContainerData { assert.NoError(err, "unable to start fake container workload %+v: %s", workload, err) // temporal dir to place container files + // Note - it is returned to the caller, who does the defer remove to clean up. tmpdir, err := ioutil.TempDir("", "") assert.NoError(err)