From d97b754a5b607a0ee2a2c2eb460dc49c455c2b2d Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Mon, 27 Jun 2022 11:54:45 -0700 Subject: [PATCH] Cleanup metadata tests This commit replaces func returns with t.Cleanup, which makes API and tests slightly easier to maintain. Signed-off-by: Maksym Pavlenko --- metadata/containers_test.go | 43 +++++++++++++-------------------- metadata/content_test.go | 10 +++----- metadata/db_test.go | 48 ++++++++++++++----------------------- metadata/gc_test.go | 46 ++++++++++++++++------------------- metadata/images_test.go | 8 +++---- metadata/leases_test.go | 9 +++---- metadata/namespaces_test.go | 3 +-- metadata/sandbox_test.go | 25 ++++++------------- metadata/snapshot_test.go | 4 +--- 9 files changed, 73 insertions(+), 123 deletions(-) diff --git a/metadata/containers_test.go b/metadata/containers_test.go index 8e75895e6..662d79cec 100644 --- 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 index 8e716aebd..8803150bc 100644 --- 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 index 4ff540fef..fd0b12aad 100644 --- 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 index a27210a2b..fbd994e53 100644 --- 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 index 323426da1..4644e0018 100644 --- 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 index be120ce77..56aa0d9f0 100644 --- 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 index 6a64c69d0..83e620c45 100644 --- 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 index 68b1dc68d..f822bd35e 100644 --- a/metadata/sandbox_test.go +++ b/metadata/sandbox_test.go @@ -27,8 +27,7 @@ import ( ) func TestSandboxCreate(t *testing.T) { - ctx, db, done := testDB(t) - defer done() + ctx, db := testDB(t) store := NewSandboxStore(db) @@ -60,8 +59,7 @@ func TestSandboxCreate(t *testing.T) { } func TestSandboxCreateDup(t *testing.T) { - ctx, db, done := testDB(t) - defer done() + ctx, db := testDB(t) store := NewSandboxStore(db) @@ -83,9 +81,7 @@ func TestSandboxCreateDup(t *testing.T) { } func TestSandboxUpdate(t *testing.T) { - ctx, db, done := testDB(t) - defer done() - + ctx, db := testDB(t) store := NewSandboxStore(db) if _, err := store.Create(ctx, api.Sandbox{ @@ -131,9 +127,7 @@ func TestSandboxUpdate(t *testing.T) { } func TestSandboxGetInvalid(t *testing.T) { - ctx, db, done := testDB(t) - defer done() - + ctx, db := testDB(t) store := NewSandboxStore(db) _, err := store.Get(ctx, "invalid_id") @@ -145,9 +139,7 @@ func TestSandboxGetInvalid(t *testing.T) { } func TestSandboxList(t *testing.T) { - ctx, db, done := testDB(t) - defer done() - + ctx, db := testDB(t) store := NewSandboxStore(db) in := []api.Sandbox{ @@ -192,9 +184,7 @@ func TestSandboxList(t *testing.T) { } func TestSandboxListWithFilter(t *testing.T) { - ctx, db, done := testDB(t) - defer done() - + ctx, db := testDB(t) store := NewSandboxStore(db) in := []api.Sandbox{ @@ -237,8 +227,7 @@ func TestSandboxListWithFilter(t *testing.T) { } func TestSandboxDelete(t *testing.T) { - ctx, db, done := testDB(t) - defer done() + ctx, db := testDB(t) store := NewSandboxStore(db) diff --git a/metadata/snapshot_test.go b/metadata/snapshot_test.go index 9cfb205e7..583794ed2 100644 --- a/metadata/snapshot_test.go +++ b/metadata/snapshot_test.go @@ -73,11 +73,9 @@ func TestMetadata(t *testing.T) { } func TestSnapshotterWithRef(t *testing.T) { - ctx, db, done := testDB(t, withSnapshotter("tmp", func(string) (snapshots.Snapshotter, error) { + ctx, db := testDB(t, withSnapshotter("tmp", func(string) (snapshots.Snapshotter, error) { return NewTmpSnapshotter(), nil })) - defer done() - sn := db.Snapshotter("tmp") test1opt := snapshots.WithLabels(