From dd6e8eb82c7ce7596b2786e2c07436e2ca7d7031 Mon Sep 17 00:00:00 2001
From: Wei Zhang <zhangwei555@huawei.com>
Date: Tue, 26 Mar 2019 16:18:16 +0800
Subject: [PATCH] tests: do cleanUp() always in the end

Fixes: #1422

Detect failing test case:

```
....
=== RUN   TestEnterContainerFailingContNotStarted
--- PASS: TestEnterContainerFailingContNotStarted (0.01s)
=== RUN   TestEnterContainer
--- FAIL: TestEnterContainer (0.00s)
 Error Trace: sandbox_test.go:1154
 Error:      	Expected value not to be nil.
 Messages:   	Entering non-running container should fail
 Error Trace: sandbox_test.go:1157
 Error:      	Expected nil, but got: &errors.errorString{s:"Can not
move from running to running"}
 Messages:   	Failed to start sandbox: Can not move from running to
running
FAIL
```

`TestEnterContainerFailingContNotStarted` calls `cleanUp` at function
begging but it doesn't clean its garbage after it ends.
`TestEnterContainer` only call `cleanUp` in the end but it doesn't do
cleanUp in the begging, that gives first test case a chance to impact
latter one.

This commit modifies all the test cases, let them all do the cleanUp()
in the end.

The policy here is: "everyone needs to take their garbage away when they
leave" :)

Signed-off-by: Wei Zhang <zhangwei555@huawei.com>
---
 virtcontainers/api_test.go        | 122 +++++++++++++++---------------
 virtcontainers/cc_shim_test.go    |   2 +-
 virtcontainers/kata_shim_test.go  |   2 +-
 virtcontainers/noop_agent_test.go |   2 +-
 virtcontainers/sandbox_test.go    |   2 +-
 5 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go
index 8e27d727cd..8f10dc9987 100644
--- a/virtcontainers/api_test.go
+++ b/virtcontainers/api_test.go
@@ -199,7 +199,7 @@ func newTestSandboxConfigKataAgent() SandboxConfig {
 }
 
 func TestCreateSandboxNoopAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -232,7 +232,7 @@ func TestCreateSandboxHyperstartAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigHyperstartAgent()
 
@@ -265,7 +265,7 @@ func TestCreateSandboxKataAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigKataAgent()
 
@@ -302,7 +302,7 @@ func TestCreateSandboxKataAgentSuccessful(t *testing.T) {
 }
 
 func TestCreateSandboxFailing(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := SandboxConfig{}
 
@@ -313,7 +313,7 @@ func TestCreateSandboxFailing(t *testing.T) {
 }
 
 func TestDeleteSandboxNoopAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	ctx := context.Background()
 	config := newTestSandboxConfigNoop()
@@ -345,7 +345,7 @@ func TestDeleteSandboxHyperstartAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigHyperstartAgent()
 
@@ -390,7 +390,7 @@ func TestDeleteSandboxKataAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigKataAgent()
 
@@ -438,7 +438,7 @@ func TestDeleteSandboxKataAgentSuccessful(t *testing.T) {
 }
 
 func TestDeleteSandboxFailing(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	sandboxDir := store.SandboxConfigurationRootPath(testSandboxID)
 	os.Remove(sandboxDir)
@@ -450,7 +450,7 @@ func TestDeleteSandboxFailing(t *testing.T) {
 }
 
 func TestStartSandboxNoopAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -461,7 +461,7 @@ func TestStartSandboxNoopAgentSuccessful(t *testing.T) {
 }
 
 func TestStartSandboxHyperstartAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	if os.Geteuid() != 0 {
 		t.Skip(testDisabledAsNonRoot)
@@ -501,7 +501,7 @@ func TestStartSandboxKataAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigKataAgent()
 
@@ -538,7 +538,7 @@ func TestStartSandboxKataAgentSuccessful(t *testing.T) {
 }
 
 func TestStartSandboxFailing(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	sandboxDir := store.SandboxConfigurationRootPath(testSandboxID)
 	os.Remove(sandboxDir)
@@ -550,7 +550,7 @@ func TestStartSandboxFailing(t *testing.T) {
 }
 
 func TestStopSandboxNoopAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -567,7 +567,7 @@ func TestStopSandboxNoopAgentSuccessful(t *testing.T) {
 }
 
 func TestPauseThenResumeSandboxNoopAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -632,7 +632,7 @@ func TestStopSandboxHyperstartAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigHyperstartAgent()
 
@@ -668,7 +668,7 @@ func TestStopSandboxKataAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigKataAgent()
 
@@ -705,7 +705,7 @@ func TestStopSandboxKataAgentSuccessful(t *testing.T) {
 }
 
 func TestStopSandboxFailing(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	sandboxDir := store.SandboxConfigurationRootPath(testSandboxID)
 	os.Remove(sandboxDir)
@@ -717,7 +717,7 @@ func TestStopSandboxFailing(t *testing.T) {
 }
 
 func TestRunSandboxNoopAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -738,7 +738,7 @@ func TestRunSandboxHyperstartAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigHyperstartAgent()
 
@@ -780,7 +780,7 @@ func TestRunSandboxKataAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigKataAgent()
 
@@ -823,7 +823,7 @@ func TestRunSandboxKataAgentSuccessful(t *testing.T) {
 }
 
 func TestRunSandboxFailing(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := SandboxConfig{}
 
@@ -834,7 +834,7 @@ func TestRunSandboxFailing(t *testing.T) {
 }
 
 func TestListSandboxSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -851,7 +851,7 @@ func TestListSandboxSuccessful(t *testing.T) {
 }
 
 func TestListSandboxNoSandboxDirectory(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	_, err := ListSandbox(context.Background())
 	if err != nil {
@@ -860,7 +860,7 @@ func TestListSandboxNoSandboxDirectory(t *testing.T) {
 }
 
 func TestStatusSandboxSuccessfulStateReady(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 	hypervisorConfig := HypervisorConfig{
@@ -919,7 +919,7 @@ func TestStatusSandboxSuccessfulStateReady(t *testing.T) {
 }
 
 func TestStatusSandboxSuccessfulStateRunning(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 	hypervisorConfig := HypervisorConfig{
@@ -983,7 +983,7 @@ func TestStatusSandboxSuccessfulStateRunning(t *testing.T) {
 }
 
 func TestStatusSandboxFailingFetchSandboxConfig(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -1003,7 +1003,7 @@ func TestStatusSandboxFailingFetchSandboxConfig(t *testing.T) {
 }
 
 func TestStatusPodSandboxFailingFetchSandboxState(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -1035,7 +1035,7 @@ func newTestContainerConfigNoop(contID string) ContainerConfig {
 }
 
 func TestCreateContainerSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1067,7 +1067,7 @@ func TestCreateContainerSuccessful(t *testing.T) {
 }
 
 func TestCreateContainerFailingNoSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1098,7 +1098,7 @@ func TestCreateContainerFailingNoSandbox(t *testing.T) {
 }
 
 func TestDeleteContainerSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1140,7 +1140,7 @@ func TestDeleteContainerSuccessful(t *testing.T) {
 }
 
 func TestDeleteContainerFailingNoSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	c, err := DeleteContainer(context.Background(), testSandboxID, contID)
@@ -1150,7 +1150,7 @@ func TestDeleteContainerFailingNoSandbox(t *testing.T) {
 }
 
 func TestDeleteContainerFailingNoContainer(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1174,7 +1174,7 @@ func TestDeleteContainerFailingNoContainer(t *testing.T) {
 }
 
 func TestStartContainerNoopAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1205,7 +1205,7 @@ func TestStartContainerNoopAgentSuccessful(t *testing.T) {
 }
 
 func TestStartContainerFailingNoSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	c, err := StartContainer(context.Background(), testSandboxID, contID)
@@ -1215,7 +1215,7 @@ func TestStartContainerFailingNoSandbox(t *testing.T) {
 }
 
 func TestStartContainerFailingNoContainer(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1239,7 +1239,7 @@ func TestStartContainerFailingNoContainer(t *testing.T) {
 }
 
 func TestStartContainerFailingSandboxNotStarted(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1276,7 +1276,7 @@ func TestStartContainerFailingSandboxNotStarted(t *testing.T) {
 }
 
 func TestStopContainerNoopAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1317,7 +1317,7 @@ func TestStartStopContainerHyperstartAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigHyperstartAgent()
@@ -1378,7 +1378,7 @@ func TestStartStopSandboxHyperstartAgentSuccessfulWithDefaultNetwork(t *testing.
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigHyperstartAgentDefaultNetwork()
 
@@ -1425,7 +1425,7 @@ func TestStartStopSandboxHyperstartAgentSuccessfulWithDefaultNetwork(t *testing.
 }
 
 func TestStopContainerFailingNoSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	c, err := StopContainer(context.Background(), testSandboxID, contID)
@@ -1435,7 +1435,7 @@ func TestStopContainerFailingNoSandbox(t *testing.T) {
 }
 
 func TestStopContainerFailingNoContainer(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1459,7 +1459,7 @@ func TestStopContainerFailingNoContainer(t *testing.T) {
 }
 
 func testKillContainerFromContReadySuccessful(t *testing.T, signal syscall.Signal) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1501,7 +1501,7 @@ func TestKillContainerFromContReadySuccessful(t *testing.T) {
 }
 
 func TestEnterContainerNoopAgentSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1544,7 +1544,7 @@ func TestEnterContainerHyperstartAgentSuccessful(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigHyperstartAgent()
@@ -1608,7 +1608,7 @@ func TestEnterContainerHyperstartAgentSuccessful(t *testing.T) {
 }
 
 func TestEnterContainerFailingNoSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 	contID := "100"
 	cmd := newBasicTestCmd()
 
@@ -1619,7 +1619,7 @@ func TestEnterContainerFailingNoSandbox(t *testing.T) {
 }
 
 func TestEnterContainerFailingNoContainer(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1645,7 +1645,7 @@ func TestEnterContainerFailingNoContainer(t *testing.T) {
 }
 
 func TestEnterContainerFailingContNotStarted(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1679,7 +1679,7 @@ func TestEnterContainerFailingContNotStarted(t *testing.T) {
 }
 
 func TestStatusContainerSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1730,7 +1730,7 @@ func TestStatusContainerSuccessful(t *testing.T) {
 }
 
 func TestStatusContainerStateReady(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	// (homage to a great album! ;)
 	contID := "101"
@@ -1796,7 +1796,7 @@ func TestStatusContainerStateReady(t *testing.T) {
 }
 
 func TestStatusContainerStateRunning(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	// (homage to a great album! ;)
 	contID := "101"
@@ -1872,7 +1872,7 @@ func TestStatusContainerStateRunning(t *testing.T) {
 }
 
 func TestStatusContainerFailing(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1893,7 +1893,7 @@ func TestStatusContainerFailing(t *testing.T) {
 }
 
 func TestStatsContainerFailing(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
@@ -1914,7 +1914,7 @@ func TestStatsContainerFailing(t *testing.T) {
 }
 
 func TestStatsContainer(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	assert := assert.New(t)
 	contID := "100"
@@ -1960,7 +1960,7 @@ func TestStatsContainer(t *testing.T) {
 }
 
 func TestProcessListContainer(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	assert := assert.New(t)
 
@@ -2255,7 +2255,7 @@ func BenchmarkStartStop10ContainerQemuHypervisorHyperstartAgentNetworkNoop(b *te
 }
 
 func TestFetchSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -2272,7 +2272,7 @@ func TestFetchSandbox(t *testing.T) {
 }
 
 func TestFetchStatefulSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -2291,14 +2291,14 @@ func TestFetchStatefulSandbox(t *testing.T) {
 }
 
 func TestFetchNonExistingSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	_, err := FetchSandbox(context.Background(), "some-non-existing-sandbox-name")
 	assert.NotNil(t, err, "fetch non-existing sandbox should fail")
 }
 
 func TestReleaseSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	config := newTestSandboxConfigNoop()
 
@@ -2315,7 +2315,7 @@ func TestUpdateContainer(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	ctx := context.Background()
 
@@ -2368,7 +2368,7 @@ func TestPauseResumeContainer(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	ctx := context.Background()
 
@@ -2410,7 +2410,7 @@ func TestNetworkOperation(t *testing.T) {
 		t.Skip(testDisabledAsNonRoot)
 	}
 
-	cleanUp()
+	defer cleanUp()
 
 	assert := assert.New(t)
 	inf := &vcTypes.Interface{
diff --git a/virtcontainers/cc_shim_test.go b/virtcontainers/cc_shim_test.go
index 9a1e1045e9..3c6733998c 100644
--- a/virtcontainers/cc_shim_test.go
+++ b/virtcontainers/cc_shim_test.go
@@ -327,7 +327,7 @@ func newConsole() (*os.File, string, error) {
 }
 
 func TestCCShimStartWithConsoleSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	master, console, err := newConsole()
 	t.Logf("Console created for tests:%s\n", console)
diff --git a/virtcontainers/kata_shim_test.go b/virtcontainers/kata_shim_test.go
index f0f16e448f..ec77383a2b 100644
--- a/virtcontainers/kata_shim_test.go
+++ b/virtcontainers/kata_shim_test.go
@@ -273,7 +273,7 @@ func TestKataShimStartWithConsoleNonExistingFailure(t *testing.T) {
 }
 
 func TestKataShimStartWithConsoleSuccessful(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 
 	master, console, err := newConsole()
 	t.Logf("Console created for tests:%s\n", console)
diff --git a/virtcontainers/noop_agent_test.go b/virtcontainers/noop_agent_test.go
index ad471f4647..e62cbe0594 100644
--- a/virtcontainers/noop_agent_test.go
+++ b/virtcontainers/noop_agent_test.go
@@ -15,7 +15,7 @@ import (
 )
 
 func testCreateNoopContainer() (*Sandbox, *Container, error) {
-	cleanUp()
+	defer cleanUp()
 
 	contID := "100"
 	config := newTestSandboxConfigNoop()
diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go
index 2fbb7908f2..109b1609a7 100644
--- a/virtcontainers/sandbox_test.go
+++ b/virtcontainers/sandbox_test.go
@@ -1615,7 +1615,7 @@ func checkSandboxRemains() error {
 }
 
 func TestSandboxCreationFromConfigRollbackFromCreateSandbox(t *testing.T) {
-	cleanUp()
+	defer cleanUp()
 	assert := assert.New(t)
 	ctx := context.Background()
 	hConf := newHypervisorConfig(nil, nil)