Check timestamps in snapshot storage test suite

also use t.Helper()
convert assertions to canonical

Signed-off-by: Daniel Nephin <dnephin@gmail.com>

cio/io_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	"github.com/containerd/fifo"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/gotestyourself/gotestyourself/assert"
 	is "github.com/gotestyourself/gotestyourself/assert/cmp"
 )
@@ -38,13 +39,17 @@ func TestNewFIFOSetInDir(t *testing.T) {
 	fifos, err := NewFIFOSetInDir(root, "theid", true)
 	assert.NilError(t, err)
 
-	assertHasPrefix(t, fifos.Stdin, root)
+	dir := filepath.Dir(fifos.Stdin)
-	assertHasPrefix(t, fifos.Stdout, root)
+	assertHasPrefix(t, dir, root)
-	assertHasPrefix(t, fifos.Stderr, root)
+	expected := &FIFOSet{
-	assert.Check(t, is.Equal("theid-stdin", filepath.Base(fifos.Stdin)))
+		Config: Config{
-	assert.Check(t, is.Equal("theid-stdout", filepath.Base(fifos.Stdout)))
+			Stdin:    filepath.Join(dir, "theid-stdin"),
-	assert.Check(t, is.Equal("theid-stderr", filepath.Base(fifos.Stderr)))
+			Stdout:   filepath.Join(dir, "theid-stdout"),
-	assert.Check(t, fifos.Terminal)
+			Stderr:   filepath.Join(dir, "theid-stderr"),
+			Terminal: true,
+		},
+	}
+	assert.Assert(t, is.DeepEqual(fifos, expected, cmpFIFOSet))
 
 	files, err := ioutil.ReadDir(root)
 	assert.NilError(t, err)
@@ -56,6 +61,8 @@ func TestNewFIFOSetInDir(t *testing.T) {
 	assert.Check(t, is.Len(files, 0))
 }
 
+var cmpFIFOSet = cmpopts.IgnoreUnexported(FIFOSet{})
+
 func TestNewAttach(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("setupFIFOProducers not yet implemented on windows")
@@ -91,9 +98,9 @@ func TestNewAttach(t *testing.T) {
 	io.Cancel()
 	assert.NilError(t, io.Close())
 
-	assert.Check(t, is.EqualMultiLine(expectedStdout, stdout.String()))
+	assert.Check(t, is.Equal(expectedStdout, stdout.String()))
-	assert.Check(t, is.EqualMultiLine(expectedStderr, stderr.String()))
+	assert.Check(t, is.Equal(expectedStderr, stderr.String()))
-	assert.Check(t, is.EqualMultiLine(expectedStdin, string(actualStdin)))
+	assert.Check(t, is.Equal(expectedStdin, string(actualStdin)))
 }
 
 type producers struct {

images/oci/importer_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"github.com/gotestyourself/gotestyourself/assert"
-	is "github.com/gotestyourself/gotestyourself/assert/cmp"
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
@@ -32,7 +31,7 @@ func TestNormalizeImageRef(t *testing.T) {
 		},
 	} {
 		normalized, err := normalizeImageRef(imageBaseName, test.input)
-		assert.Check(t, is.NilError(err))
+		assert.NilError(t, err)
-		assert.Check(t, is.Equal(test.expect, normalized))
+		assert.Equal(t, test.expect, normalized)
 	}
 }

mount/lookup_test/lookup_linux_test.go

@@ -21,14 +21,14 @@ import (
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/testutil"
 	"github.com/gotestyourself/gotestyourself/assert"
-	is "github.com/gotestyourself/gotestyourself/assert/cmp"
 )
 
 func checkLookup(t *testing.T, fsType, mntPoint, dir string) {
+	t.Helper()
 	info, err := mount.Lookup(dir)
 	assert.NilError(t, err)
-	assert.Check(t, is.Equal(fsType, info.FSType))
+	assert.Equal(t, fsType, info.FSType)
-	assert.Check(t, is.Equal(mntPoint, info.Mountpoint))
+	assert.Equal(t, mntPoint, info.Mountpoint)
 }
 
 func testLookup(t *testing.T, fsType string) {

remotes/docker/scope_test.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/containerd/containerd/reference"
 	"github.com/gotestyourself/gotestyourself/assert"
-	is "github.com/gotestyourself/gotestyourself/assert/cmp"
 )
 
 func TestRepositoryScope(t *testing.T) {
@@ -32,8 +31,10 @@ func TestRepositoryScope(t *testing.T) {
 		},
 	}
 	for _, x := range testCases {
-		actual, err := repositoryScope(x.refspec, x.push)
+		t.Run(x.refspec.String(), func(t *testing.T) {
-		assert.Check(t, is.NilError(err))
+			actual, err := repositoryScope(x.refspec, x.push)
-		assert.Check(t, is.Equal(x.expected, actual))
+			assert.NilError(t, err)
+			assert.Equal(t, x.expected, actual)
+		})
 	}
 }

snapshots/storage/metastore_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/snapshots"
+	"github.com/google/go-cmp/cmp"
 	"github.com/gotestyourself/gotestyourself/assert"
 	is "github.com/gotestyourself/gotestyourself/assert/cmp"
 	"github.com/pkg/errors"
@@ -199,75 +200,71 @@ var baseInfo = map[string]snapshots.Info{
 }
 
 func assertNotExist(t *testing.T, err error) {
-	if err == nil {
+	t.Helper()
-		t.Fatal("Expected not exist error")
+	assert.Assert(t, errdefs.IsNotFound(err), "got %+v", err)
-	}
-	if !errdefs.IsNotFound(err) {
-		t.Fatalf("Expected not exist error, got %+v", err)
-	}
 }
 
 func assertNotActive(t *testing.T, err error) {
-	if err == nil {
+	t.Helper()
-		t.Fatal("Expected not active error")
+	assert.Assert(t, errdefs.IsFailedPrecondition(err), "got %+v", err)
-	}
-	if !errdefs.IsFailedPrecondition(err) {
-		t.Fatalf("Expected not active error, got %+v", err)
-	}
 }
 
 func assertNotCommitted(t *testing.T, err error) {
-	if err == nil {
+	t.Helper()
-		t.Fatal("Expected active error")
+	assert.Assert(t, errdefs.IsInvalidArgument(err), "got %+v", err)
-	}
-	if !errdefs.IsInvalidArgument(err) {
-		t.Fatalf("Expected active error, got %+v", err)
-	}
 }
 
 func assertExist(t *testing.T, err error) {
-	if err == nil {
+	t.Helper()
-		t.Fatal("Expected exist error")
+	assert.Assert(t, errdefs.IsAlreadyExists(err), "got %+v", err)
-	}
-	if !errdefs.IsAlreadyExists(err) {
-		t.Fatalf("Expected exist error, got %+v", err)
-	}
 }
 
-func testGetInfo(ctx context.Context, t *testing.T, ms *MetaStore) {
+func testGetInfo(ctx context.Context, t *testing.T, _ *MetaStore) {
 	for key, expected := range baseInfo {
 		_, info, _, err := GetInfo(ctx, key)
-		if err != nil {
+		assert.NilError(t, err, "on key %v", key)
-			t.Fatalf("GetInfo on %v failed: %+v", key, err)
+		assert.Check(t, is.DeepEqual(expected, info, cmpSnapshotInfo), "on key %v", key)
-		}
-		// TODO: Check timestamp range
-		info.Created = time.Time{}
-		info.Updated = time.Time{}
-		assert.Check(t, is.DeepEqual(expected, info))
 	}
 }
 
-func testGetInfoNotExist(ctx context.Context, t *testing.T, ms *MetaStore) {
+// compare snapshot.Info Updated and Created fields by checking they are
+// within a threshold of time.Now()
+var cmpSnapshotInfo = cmp.FilterPath(
+	func(path cmp.Path) bool {
+		field := path.Last().String()
+		return field == ".Created" || field == ".Updated"
+	},
+	cmp.Comparer(func(expected, actual time.Time) bool {
+		// cmp.Options must be symmetric, so swap the args
+		if actual.IsZero() {
+			actual, expected = expected, actual
+		}
+		if !expected.IsZero() {
+			return false
+		}
+		// actual value should be within a few seconds of now
+		now := time.Now()
+		delta := now.Sub(actual)
+		threshold := 10 * time.Second
+		return delta > -threshold && delta < threshold
+	}))
+
+func testGetInfoNotExist(ctx context.Context, t *testing.T, _ *MetaStore) {
 	_, _, _, err := GetInfo(ctx, "active-not-exist")
 	assertNotExist(t, err)
 }
 
-func testWalk(ctx context.Context, t *testing.T, ms *MetaStore) {
+func testWalk(ctx context.Context, t *testing.T, _ *MetaStore) {
 	found := map[string]snapshots.Info{}
 	err := WalkInfo(ctx, func(ctx context.Context, info snapshots.Info) error {
 		if _, ok := found[info.Name]; ok {
 			return errors.Errorf("entry already encountered")
 		}
-		// TODO: Check time range
-		info.Created = time.Time{}
-		info.Updated = time.Time{}
 		found[info.Name] = info
 		return nil
 	})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf("Walk failed: %+v", err)
+	assert.Assert(t, is.DeepEqual(baseInfo, found, cmpSnapshotInfo))
-	}
-	assert.Check(t, is.DeepEqual(baseInfo, found))
 }
 
 func testGetSnapshot(ctx context.Context, t *testing.T, ms *MetaStore) {
@@ -316,10 +313,8 @@ func testGetSnapshot(ctx context.Context, t *testing.T, ms *MetaStore) {
 	test := func(ctx context.Context, t *testing.T, ms *MetaStore) {
 		for key, expected := range snapshotMap {
 			s, err := GetSnapshot(ctx, key)
-			if err != nil {
+			assert.NilError(t, err, "failed to get snapshot %s", key)
-				t.Fatalf("Failed to get active: %+v", err)
+			assert.Check(t, is.DeepEqual(expected, s), "on key %s", key)
-			}
-			assert.Check(t, is.DeepEqual(expected, s))
 		}
 	}
 
@@ -546,7 +541,7 @@ func testRemoveWithChildren(ctx context.Context, t *testing.T, ms *MetaStore) {
 	}
 }
 
-func testRemoveNotExist(ctx context.Context, t *testing.T, ms *MetaStore) {
+func testRemoveNotExist(ctx context.Context, t *testing.T, _ *MetaStore) {
 	_, _, err := Remove(ctx, "does-not-exist")
 	assertNotExist(t, err)
 }

snapshots/testsuite/testsuite.go

@@ -207,10 +207,10 @@ func checkSnapshotterBasic(ctx context.Context, t *testing.T, snapshotter snapsh
 		si2.Name: si2,
 	}
 	walked := map[string]snapshots.Info{} // walk is not ordered
-	assert.Check(t, is.NilError(snapshotter.Walk(ctx, func(ctx context.Context, si snapshots.Info) error {
+	assert.NilError(t, snapshotter.Walk(ctx, func(ctx context.Context, si snapshots.Info) error {
 		walked[si.Name] = si
 		return nil
-	})))
+	}))
 
 	for ek, ev := range expected {
 		av, ok := walked[ek]
@@ -241,10 +241,10 @@ func checkSnapshotterBasic(ctx context.Context, t *testing.T, snapshotter snapsh
 	}
 
 	testutil.Unmount(t, nextnext)
-	assert.Check(t, is.NilError(snapshotter.Remove(ctx, nextnext)))
+	assert.NilError(t, snapshotter.Remove(ctx, nextnext))
-	assert.Check(t, is.ErrorContains(snapshotter.Remove(ctx, committed), ""))
+	assert.Assert(t, is.ErrorContains(snapshotter.Remove(ctx, committed), "remove"))
-	assert.Check(t, is.NilError(snapshotter.Remove(ctx, nextCommitted)))
+	assert.NilError(t, snapshotter.Remove(ctx, nextCommitted))
-	assert.Check(t, is.NilError(snapshotter.Remove(ctx, committed)))
+	assert.NilError(t, snapshotter.Remove(ctx, committed))
 }
 
 // Create a New Layer on top of base layer with Prepare, Stat on new layer, should return Active layer.
@@ -779,8 +779,8 @@ func checkSnapshotterViewReadonly(ctx context.Context, t *testing.T, snapshotter
 		t.Fatalf("write to %q should fail (EROFS) but did not fail", testfile)
 	}
 	testutil.Unmount(t, viewMountPoint)
-	assert.Check(t, is.NilError(snapshotter.Remove(ctx, view)))
+	assert.NilError(t, snapshotter.Remove(ctx, view))
-	assert.Check(t, is.NilError(snapshotter.Remove(ctx, committed)))
+	assert.NilError(t, snapshotter.Remove(ctx, committed))
 }
 
 // Move files from base layer to new location in intermediate layer.