Merge pull request #7105 from mxpv/cleanup

dmcgowan · web-flow · commit b76544b8f507 · 2022-06-27T16:23:24.000-07:00
Cleanup metadata tests
diff --git a/metadata/containers_test.go b/metadata/containers_test.go
@@ -35,6 +35,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	bolt "go.etcd.io/bbolt"
 )
 
@@ -43,16 +44,11 @@ func init() {
 }
 
 func TestContainersList(t *testing.T) {
-	ctx, db, cancel := testEnv(t)
-	defer cancel()
-
+	ctx, db := testEnv(t)
 	store := NewContainerStore(NewDB(db, nil, nil))
-
 	spec := &specs.Spec{}
 	encoded, err := protobuf.MarshalAnyToProto(spec)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	testset := map[string]*containers.Container{}
 	for i := 0; i < 4; i++ {
@@ -175,22 +171,18 @@ func TestContainersList(t *testing.T) {
 
 // TestContainersUpdate ensures that updates are taken in an expected manner.
 func TestContainersCreateUpdateDelete(t *testing.T) {
-	ctx, db, cancel := testEnv(t)
-	defer cancel()
+	var (
+		ctx, db = testEnv(t)
+		store   = NewContainerStore(NewDB(db, nil, nil))
+		spec    = &specs.Spec{}
+	)
 
-	store := NewContainerStore(NewDB(db, nil, nil))
-
-	spec := &specs.Spec{}
 	encoded, err := protobuf.MarshalAnyToProto(spec)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	spec.Annotations = map[string]string{"updated": "true"}
 	encodedUpdated, err := protobuf.MarshalAnyToProto(spec)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	for _, testcase := range []struct {
 		name       string
@@ -708,20 +700,19 @@ func checkContainersEqual(t *testing.T, a, b *containers.Container, format strin
 	assert.True(t, cmp.Equal(a, b, compareNil, compareAny))
 }
 
-func testEnv(t *testing.T) (context.Context, *bolt.DB, func()) {
+func testEnv(t *testing.T) (context.Context, *bolt.DB) {
 	ctx, cancel := context.WithCancel(context.Background())
 	ctx = namespaces.WithNamespace(ctx, "testing")
 	ctx = logtest.WithT(ctx, t)
-
 	dirname := t.TempDir()
 
 	db, err := bolt.Open(filepath.Join(dirname, "meta.db"), 0644, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
-	return ctx, db, func() {
-		db.Close()
+	t.Cleanup(func() {
+		assert.NoError(t, db.Close())
 		cancel()
-	}
+	})
+
+	return ctx, db
 }
diff --git a/metadata/content_test.go b/metadata/content_test.go
@@ -32,7 +32,7 @@ import (
 	"github.com/containerd/containerd/labels"
 	"github.com/containerd/containerd/leases"
 	"github.com/containerd/containerd/namespaces"
-	digest "github.com/opencontainers/go-digest"
+	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	bolt "go.etcd.io/bbolt"
 )
@@ -95,8 +95,7 @@ func TestContent(t *testing.T) {
 }
 
 func TestContentLeased(t *testing.T) {
-	ctx, db, cancel := testDB(t)
-	defer cancel()
+	ctx, db := testDB(t)
 
 	cs := db.ContentStore()
 
@@ -143,11 +142,8 @@ func TestContentLeased(t *testing.T) {
 }
 
 func TestIngestLeased(t *testing.T) {
-	ctx, db, cancel := testDB(t)
-	defer cancel()
-
+	ctx, db := testDB(t)
 	cs := db.ContentStore()
-
 	blob := []byte("any content")
 	expected := digest.FromBytes(blob)
 
diff --git a/metadata/db_test.go b/metadata/db_test.go
@@ -41,8 +41,10 @@ import (
 	"github.com/containerd/containerd/protobuf/types"
 	"github.com/containerd/containerd/snapshots"
 	"github.com/containerd/containerd/snapshots/native"
-	digest "github.com/opencontainers/go-digest"
+	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	bolt "go.etcd.io/bbolt"
 )
 
@@ -61,7 +63,7 @@ func withSnapshotter(name string, fn func(string) (snapshots.Snapshotter, error)
 	}
 }
 
-func testDB(t *testing.T, opt ...testOpt) (context.Context, *DB, func()) {
+func testDB(t *testing.T, opt ...testOpt) (context.Context, *DB) {
 	ctx, cancel := context.WithCancel(context.Background())
 	ctx = namespaces.WithNamespace(ctx, "testing")
 	ctx = logtest.WithT(ctx, t)
@@ -75,9 +77,7 @@ func testDB(t *testing.T, opt ...testOpt) (context.Context, *DB, func()) {
 	dirname := t.TempDir()
 
 	snapshotter, err := native.NewSnapshotter(filepath.Join(dirname, "native"))
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	snapshotters := map[string]snapshots.Snapshotter{
 		"native": snapshotter,
@@ -92,41 +92,30 @@ func testDB(t *testing.T, opt ...testOpt) (context.Context, *DB, func()) {
 	}
 
 	cs, err := local.NewStore(filepath.Join(dirname, "content"))
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	bdb, err := bolt.Open(filepath.Join(dirname, "metadata.db"), 0644, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	db := NewDB(bdb, cs, snapshotters)
-	if err := db.Init(ctx); err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, db.Init(ctx))
 
-	return ctx, db, func() {
-		bdb.Close()
+	t.Cleanup(func() {
+		assert.NoError(t, bdb.Close())
 		cancel()
-	}
+	})
+
+	return ctx, db
 }
 
 func TestInit(t *testing.T) {
-	ctx, db, cancel := testEnv(t)
-	defer cancel()
+	ctx, db := testEnv(t)
 
-	if err := NewDB(db, nil, nil).Init(ctx); err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, NewDB(db, nil, nil).Init(ctx))
 
 	version, err := readDBVersion(db, bucketKeyVersion)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if version != dbVersion {
-		t.Fatalf("Unexpected version %d, expected %d", version, dbVersion)
-	}
+	require.NoError(t, err)
+	assert.EqualValues(t, version, dbVersion, "unexpected version %d, expected %d", version, dbVersion)
 }
 
 func TestMigrations(t *testing.T) {
@@ -316,8 +305,7 @@ func TestMigrations(t *testing.T) {
 
 func runMigrationTest(i int, init, check func(*bolt.Tx) error) func(t *testing.T) {
 	return func(t *testing.T) {
-		_, db, cancel := testEnv(t)
-		defer cancel()
+		_, db := testEnv(t)
 
 		if err := db.Update(init); err != nil {
 			t.Fatal(err)
diff --git a/metadata/gc_test.go b/metadata/gc_test.go
@@ -27,7 +27,9 @@ import (
 
 	"github.com/containerd/containerd/gc"
 	"github.com/containerd/containerd/metadata/boltutil"
-	digest "github.com/opencontainers/go-digest"
+	"github.com/opencontainers/go-digest"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	bolt "go.etcd.io/bbolt"
 )
 
@@ -41,11 +43,8 @@ func TestResourceMax(t *testing.T) {
 }
 
 func TestGCRoots(t *testing.T) {
-	db, cleanup, err := newDatabase(t)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer cleanup()
+	db, err := newDatabase(t)
+	require.NoError(t, err)
 
 	alters := []alterFunc{
 		addImage("ns1", "image1", dgst(1), nil),
@@ -162,11 +161,8 @@ func TestGCRoots(t *testing.T) {
 }
 
 func TestGCRemove(t *testing.T) {
-	db, cleanup, err := newDatabase(t)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer cleanup()
+	db, err := newDatabase(t)
+	require.NoError(t, err)
 
 	alters := []alterFunc{
 		addImage("ns1", "image1", dgst(1), nil),
@@ -256,11 +252,8 @@ func TestGCRemove(t *testing.T) {
 }
 
 func TestGCRefs(t *testing.T) {
-	db, cleanup, err := newDatabase(t)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer cleanup()
+	db, err := newDatabase(t)
+	require.NoError(t, err)
 
 	alters := []alterFunc{
 		addContent("ns1", dgst(1), nil),
@@ -389,12 +382,11 @@ func TestGCRefs(t *testing.T) {
 }
 
 func TestCollectibleResources(t *testing.T) {
-	db, cleanup, err := newDatabase(t)
-	if err != nil {
-		t.Fatal(err)
-	}
+	db, err := newDatabase(t)
+	require.NoError(t, err)
+
 	testResource := gc.ResourceType(0x10)
-	defer cleanup()
+
 	alters := []alterFunc{
 		addContent("ns1", dgst(1), nil),
 		addImage("ns1", "image1", dgst(1), nil),
@@ -556,17 +548,19 @@ func (tc *testCollector) Finish() error {
 	return nil
 }
 
-func newDatabase(t testing.TB) (*bolt.DB, func(), error) {
+func newDatabase(t testing.TB) (*bolt.DB, error) {
 	td := t.TempDir()
 
 	db, err := bolt.Open(filepath.Join(td, "test.db"), 0777, nil)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
-	return db, func() {
-		db.Close()
-	}, nil
+	t.Cleanup(func() {
+		assert.NoError(t, db.Close())
+	})
+
+	return db, nil
 }
 
 func checkNodeC(ctx context.Context, t *testing.T, db *bolt.DB, expected []gc.Node, fn func(context.Context, *bolt.Tx, chan<- gc.Node) error) {
diff --git a/metadata/images_test.go b/metadata/images_test.go
@@ -26,13 +26,12 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/filters"
 	"github.com/containerd/containerd/images"
-	digest "github.com/opencontainers/go-digest"
+	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 func TestImagesList(t *testing.T) {
-	ctx, db, cancel := testEnv(t)
-	defer cancel()
+	ctx, db := testEnv(t)
 	store := NewImageStore(NewDB(db, nil, nil))
 
 	testset := map[string]*images.Image{}
@@ -148,8 +147,7 @@ func TestImagesList(t *testing.T) {
 	}
 }
 func TestImagesCreateUpdateDelete(t *testing.T) {
-	ctx, db, cancel := testEnv(t)
-	defer cancel()
+	ctx, db := testEnv(t)
 	store := NewImageStore(NewDB(db, nil, nil))
 
 	for _, testcase := range []struct {
diff --git a/metadata/leases_test.go b/metadata/leases_test.go
@@ -28,8 +28,7 @@ import (
 )
 
 func TestLeases(t *testing.T) {
-	ctx, db, cancel := testEnv(t)
-	defer cancel()
+	ctx, db := testEnv(t)
 
 	lm := NewLeaseManager(NewDB(db, nil, nil))
 
@@ -108,8 +107,7 @@ func TestLeases(t *testing.T) {
 }
 
 func TestLeasesList(t *testing.T) {
-	ctx, db, cancel := testEnv(t)
-	defer cancel()
+	ctx, db := testEnv(t)
 
 	lm := NewLeaseManager(NewDB(db, nil, nil))
 
@@ -252,8 +250,7 @@ func TestLeasesList(t *testing.T) {
 }
 
 func TestLeaseResource(t *testing.T) {
-	ctx, db, cancel := testEnv(t)
-	defer cancel()
+	ctx, db := testEnv(t)
 
 	lm := NewLeaseManager(NewDB(db, nil, nil))
 
diff --git a/metadata/namespaces_test.go b/metadata/namespaces_test.go
@@ -29,8 +29,7 @@ import (
 )
 
 func TestCreateDelete(t *testing.T) {
-	ctx, db, cleanup := testDB(t)
-	defer cleanup()
+	ctx, db := testDB(t)
 
 	subtests := []struct {
 		name     string
diff --git a/metadata/sandbox_test.go b/metadata/sandbox_test.go
diff --git a/metadata/snapshot_test.go b/metadata/snapshot_test.go

Original file line number	Diff line number	Diff line change
`@@ -28,8 +28,7 @@ import (`
`28`	`28`	`)`
`29`	`29`
`30`	`30`	`func TestLeases(t *testing.T) {`
`31`		`- ctx, db, cancel := testEnv(t)`
`32`		`- defer cancel()`
	`31`	`+ ctx, db := testEnv(t)`
`33`	`32`
`34`	`33`	`lm := NewLeaseManager(NewDB(db, nil, nil))`
`35`	`34`
`@@ -108,8 +107,7 @@ func TestLeases(t *testing.T) {`
`108`	`107`	`}`
`109`	`108`
`110`	`109`	`func TestLeasesList(t *testing.T) {`
`111`		`- ctx, db, cancel := testEnv(t)`
`112`		`- defer cancel()`
	`110`	`+ ctx, db := testEnv(t)`
`113`	`111`
`114`	`112`	`lm := NewLeaseManager(NewDB(db, nil, nil))`
`115`	`113`
`@@ -252,8 +250,7 @@ func TestLeasesList(t *testing.T) {`
`252`	`250`	`}`
`253`	`251`
`254`	`252`	`func TestLeaseResource(t *testing.T) {`
`255`		`- ctx, db, cancel := testEnv(t)`
`256`		`- defer cancel()`
	`253`	`+ ctx, db := testEnv(t)`
`257`	`254`
`258`	`255`	`lm := NewLeaseManager(NewDB(db, nil, nil))`
`259`	`256`
Original file line number	Diff line number	Diff line change
`@@ -29,8 +29,7 @@ import (`
`29`	`29`	`)`
`30`	`30`
`31`	`31`	`func TestCreateDelete(t *testing.T) {`
`32`		`- ctx, db, cleanup := testDB(t)`
`33`		`- defer cleanup()`
	`32`	`+ ctx, db := testDB(t)`
`34`	`33`
`35`	`34`	`subtests := []struct {`
`36`	`35`	`name string`