Merge pull request #1117 from AkihiroSuda/fix-root-test

*_test.go: fix test conditions

Makefile

@@ -121,15 +121,15 @@ test: ## run tests, except integration tests and tests that require root
 
 root-test: ## run tests, except integration tests
 	@echo "$(WHALE) $@"
-	@go test ${TESTFLAGS} ${TEST_REQUIRES_ROOT_PACKAGES} -test.root
+	@go test ${TESTFLAGS} $(filter-out ${INTEGRATION_PACKAGE},${TEST_REQUIRES_ROOT_PACKAGES}) -test.root
 
 integration: ## run integration tests
 	@echo "$(WHALE) $@"
-	@go test ${TESTFLAGS}
+	@go test ${TESTFLAGS} -test.root
 
 benchmark: ## run benchmarks tests
 	@echo "$(WHALE) $@"
-	@go test ${TESTFLAGS} -bench . -run Benchmark
+	@go test ${TESTFLAGS} -bench . -run Benchmark -test.root
 
 FORCE:
 
@@ -173,14 +173,14 @@ coverage: ## generate coverprofiles from the unit tests, except tests that requi
 
 root-coverage: ## generae coverage profiles for the unit tests
 	@echo "$(WHALE) $@"
-	@( for pkg in ${TEST_REQUIRES_ROOT_PACKAGES}; do \
+	@( for pkg in $(filter-out ${INTEGRATION_PACKAGE},${TEST_REQUIRES_ROOT_PACKAGES}); do \
 		go test -i ${TESTFLAGS} -test.short -coverprofile="../../../$$pkg/coverage.txt" -covermode=atomic $$pkg -test.root || exit; \
 		go test ${TESTFLAGS} -test.short -coverprofile="../../../$$pkg/coverage.txt" -covermode=atomic $$pkg -test.root || exit; \
 	done )
 
 coverage-integration: ## generate coverprofiles from the integration tests
 	@echo "$(WHALE) $@"
-	go test ${TESTFLAGS} -test.short -coverprofile="../../../${INTEGRATION_PACKAGE}/coverage.txt" -covermode=atomic ${INTEGRATION_PACKAGE}
+	go test ${TESTFLAGS} -test.short -coverprofile="../../../${INTEGRATION_PACKAGE}/coverage.txt" -covermode=atomic ${INTEGRATION_PACKAGE} -test.root
 
 vendor:
 	@echo "$(WHALE) $@"

benchmark_test.go

@@ -6,10 +6,7 @@ import (
 )
 
 func BenchmarkContainerCreate(b *testing.B) {
-	if testing.Short() {
+	client, err := newClient(b, address)
-		b.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		b.Fatal(err)
 	}
@@ -52,10 +49,7 @@ func BenchmarkContainerCreate(b *testing.B) {
 }
 
 func BenchmarkContainerStart(b *testing.B) {
-	if testing.Short() {
+	client, err := newClient(b, address)
-		b.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		b.Fatal(err)
 	}

checkpoint_test.go

@@ -8,13 +8,10 @@ import (
 )
 
 func TestCheckpointRestore(t *testing.T) {
-	if testing.Short() {
-		t.Skip()
-	}
 	if !supportsCriu {
 		t.Skip("system does not have criu installed")
 	}
-	client, err := New(address)
+	client, err := newClient(t, address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -103,13 +100,10 @@ func TestCheckpointRestore(t *testing.T) {
 }
 
 func TestCheckpointRestoreNewContainer(t *testing.T) {
-	if testing.Short() {
-		t.Skip()
-	}
 	if !supportsCriu {
 		t.Skip("system does not have criu installed")
 	}
-	client, err := New(address)
+	client, err := newClient(t, address)
 	if err != nil {
 		t.Fatal(err)
 	}

client_test.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/containerd/containerd/namespaces"
+	"github.com/containerd/containerd/testutil"
 )
 
 const (
@@ -41,7 +42,7 @@ func TestMain(m *testing.M) {
 	if testing.Short() {
 		os.Exit(m.Run())
 	}
-
+	testutil.RequiresRootM()
 	// check if criu is installed on the system
 	_, err := exec.LookPath("criu")
 	supportsCriu = err == nil
@@ -134,11 +135,16 @@ func waitForDaemonStart(ctx context.Context, address string) (*Client, error) {
 	return nil, fmt.Errorf("containerd did not start within 2s: %v", err)
 }
 
-func TestNewClient(t *testing.T) {
+func newClient(t testing.TB, address string, opts ...ClientOpt) (*Client, error) {
 	if testing.Short() {
 		t.Skip()
 	}
-	client, err := New(address)
+	// testutil.RequiresRoot(t) is not needed here (already called in TestMain)
+	return New(address, opts...)
+}
+
+func TestNewClient(t *testing.T) {
+	client, err := newClient(t, address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -151,18 +157,14 @@ func TestNewClient(t *testing.T) {
 }
 
 func TestImagePull(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	ctx, cancel := testContext()
-	defer cancel()
-
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer client.Close()
 
+	ctx, cancel := testContext()
+	defer cancel()
 	_, err = client.Pull(ctx, testImage)
 	if err != nil {
 		t.Error(err)

container_test.go

@@ -20,10 +20,7 @@ func empty() IOCreation {
 }
 
 func TestContainerList(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -43,11 +40,8 @@ func TestContainerList(t *testing.T) {
 }
 
 func TestNewContainer(t *testing.T) {
-	if testing.Short() {
-		t.Skip()
-	}
 	id := t.Name()
-	client, err := New(address)
+	client, err := newClient(t, address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -82,10 +76,7 @@ func TestNewContainer(t *testing.T) {
 }
 
 func TestContainerStart(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -152,10 +143,7 @@ func TestContainerStart(t *testing.T) {
 }
 
 func TestContainerOutput(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -225,10 +213,7 @@ func TestContainerOutput(t *testing.T) {
 }
 
 func TestContainerExec(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -320,10 +305,7 @@ func TestContainerExec(t *testing.T) {
 }
 
 func TestContainerProcesses(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -393,10 +375,7 @@ func TestContainerProcesses(t *testing.T) {
 }
 
 func TestContainerCloseIO(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -477,10 +456,7 @@ func TestContainerCloseIO(t *testing.T) {
 }
 
 func TestContainerAttach(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -602,10 +578,7 @@ func TestContainerAttach(t *testing.T) {
 }
 
 func TestDeleteRunningContainer(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -670,10 +643,7 @@ func TestDeleteRunningContainer(t *testing.T) {
 }
 
 func TestContainerKill(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -739,10 +709,7 @@ func TestContainerKill(t *testing.T) {
 }
 
 func TestContainerUpdate(t *testing.T) {
-	if testing.Short() {
+	client, err := newClient(t, address)
-		t.Skip()
-	}
-	client, err := New(address)
 	if err != nil {
 		t.Fatal(err)
 	}

testutil/helpers.go

@@ -2,6 +2,7 @@ package testutil
 
 import (
 	"flag"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -28,7 +29,7 @@ func Unmount(t *testing.T, mountPoint string) {
 
 // RequiresRoot skips tests that require root, unless the test.root flag has
 // been set
-func RequiresRoot(t *testing.T) {
+func RequiresRoot(t testing.TB) {
 	if !rootEnabled {
 		t.Skip("skipping test that requires root")
 		return
@@ -36,6 +37,18 @@ func RequiresRoot(t *testing.T) {
 	assert.Equal(t, 0, os.Getuid(), "This test must be run as root.")
 }
 
+// RequiresRootM is similar to RequiresRoot but intended to be called from *testing.M.
+func RequiresRootM() {
+	if !rootEnabled {
+		fmt.Fprintln(os.Stderr, "skipping test that requires root")
+		os.Exit(0)
+	}
+	if 0 != os.Getuid() {
+		fmt.Fprintln(os.Stderr, "This test must be run as root.")
+		os.Exit(1)
+	}
+}
+
 // DumpDir will log out all of the contents of the provided directory to
 // testing logger.
 //