Use test setup/teardown to reduce code duplication (#277)

Tests that run multiple sub-tests do not use the fields in the test
suite because that would cause the model store instance to be reused
instead of having a new model store instance for each subtest.

src/internal/kopia/kopia_test.go

@@ -233,6 +233,8 @@ func (suite *KopiaUnitSuite) TestBuildDirectoryTree_Fails() {
 // ---------------
 type KopiaIntegrationSuite struct {
 	suite.Suite
+	k   *KopiaWrapper
+	ctx context.Context
 }
 
 func TestKopiaIntegrationSuite(t *testing.T) {
@@ -251,6 +253,17 @@ func (suite *KopiaIntegrationSuite) SetupSuite() {
 	require.NoError(suite.T(), err)
 }
 
+func (suite *KopiaIntegrationSuite) SetupTest() {
+	suite.ctx = context.Background()
+	k, err := openKopiaRepo(suite.T(), suite.ctx)
+	require.NoError(suite.T(), err)
+	suite.k = k
+}
+
+func (suite *KopiaIntegrationSuite) TearDownTest() {
+	assert.NoError(suite.T(), suite.k.Close(suite.ctx))
+}
+
 func (suite *KopiaIntegrationSuite) TestCloseTwiceDoesNotCrash() {
 	ctx := context.Background()
 	t := suite.T()
@@ -263,15 +276,8 @@ func (suite *KopiaIntegrationSuite) TestCloseTwiceDoesNotCrash() {
 }
 
 func (suite *KopiaIntegrationSuite) TestBackupCollections() {
-	ctx := context.Background()
 	t := suite.T()
 
-	k, err := openKopiaRepo(t, ctx)
-	require.NoError(t, err)
-	defer func() {
-		assert.NoError(t, k.Close(ctx))
-	}()
-
 	collections := []connector.DataCollection{
 		mockconnector.NewMockExchangeDataCollection(
 			[]string{"a-tenant", "user1", "emails"},
@@ -283,7 +289,7 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections() {
 		),
 	}
 
-	stats, err := k.BackupCollections(ctx, collections)
+	stats, err := suite.k.BackupCollections(suite.ctx, collections)
 	assert.NoError(t, err)
 	assert.Equal(t, stats.TotalFileCount, 47)
 	assert.Equal(t, stats.TotalDirectoryCount, 5)
@@ -292,7 +298,37 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections() {
 	assert.False(t, stats.Incomplete)
 }
 
-func setupSimpleRepo(t *testing.T, ctx context.Context, k *KopiaWrapper) manifest.ID {
+type KopiaSimpleRepoIntegrationSuite struct {
+	suite.Suite
+	k          *KopiaWrapper
+	ctx        context.Context
+	snapshotID manifest.ID
+}
+
+func TestKopiaSimpleRepoIntegrationSuite(t *testing.T) {
+	if err := ctesting.RunOnAny(
+		ctesting.CorsoCITests,
+		ctesting.CorsoKopiaWrapperTests,
+	); err != nil {
+		t.Skip()
+	}
+
+	suite.Run(t, new(KopiaSimpleRepoIntegrationSuite))
+}
+
+func (suite *KopiaSimpleRepoIntegrationSuite) SetupSuite() {
+	_, err := ctesting.GetRequiredEnvVars(ctesting.AWSStorageCredEnvs...)
+	require.NoError(suite.T(), err)
+}
+
+func (suite *KopiaSimpleRepoIntegrationSuite) SetupTest() {
+	t := suite.T()
+	suite.ctx = context.Background()
+	k, err := openKopiaRepo(t, suite.ctx)
+	require.NoError(t, err)
+
+	suite.k = k
+
 	collections := []connector.DataCollection{
 		&singleItemCollection{
 			path: testPath,
@@ -303,7 +339,7 @@ func setupSimpleRepo(t *testing.T, ctx context.Context, k *KopiaWrapper) manifes
 		},
 	}
 
-	stats, err := k.BackupCollections(ctx, collections)
+	stats, err := suite.k.BackupCollections(suite.ctx, collections)
 	require.NoError(t, err)
 	require.Equal(t, stats.TotalFileCount, 1)
 	require.Equal(t, stats.TotalDirectoryCount, 3)
@@ -311,24 +347,19 @@ func setupSimpleRepo(t *testing.T, ctx context.Context, k *KopiaWrapper) manifes
 	require.Equal(t, stats.ErrorCount, 0)
 	require.False(t, stats.Incomplete)
 
-	return manifest.ID(stats.SnapshotID)
+	suite.snapshotID = manifest.ID(stats.SnapshotID)
 }
 
-func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem() {
+func (suite *KopiaSimpleRepoIntegrationSuite) TearDownTest() {
-	ctx := context.Background()
+	assert.NoError(suite.T(), suite.k.Close(suite.ctx))
+}
+
+func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem() {
 	t := suite.T()
 
-	k, err := openKopiaRepo(t, ctx)
+	c, err := suite.k.RestoreSingleItem(
-	require.NoError(t, err)
+		suite.ctx,
-	defer func() {
+		string(suite.snapshotID),
-		assert.NoError(t, k.Close(ctx))
-	}()
-
-	id := setupSimpleRepo(t, ctx, k)
-
-	c, err := k.RestoreSingleItem(
-		ctx,
-		string(id),
 		append(testPath, testFileUUID),
 	)
 	require.NoError(t, err)
@@ -349,7 +380,7 @@ func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem() {
 
 // TestBackupAndRestoreSingleItem_Errors exercises the public RestoreSingleItem
 // function.
-func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem_Errors() {
+func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem_Errors() {
 	table := []struct {
 		name           string
 		snapshotIDFunc func(manifest.ID) manifest.ID
@@ -380,20 +411,9 @@ func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem_Errors() {
 
 	for _, test := range table {
 		suite.T().Run(test.name, func(t *testing.T) {
-			ctx := context.Background()
+			_, err := suite.k.RestoreSingleItem(
-			ctesting.LogTimeOfTest(t)
+				suite.ctx,
-
+				string(test.snapshotIDFunc(suite.snapshotID)),
-			k, err := openKopiaRepo(t, ctx)
-			require.NoError(t, err)
-			defer func() {
-				assert.NoError(t, k.Close(ctx))
-			}()
-
-			id := setupSimpleRepo(t, ctx, k)
-
-			_, err = k.RestoreSingleItem(
-				ctx,
-				string(test.snapshotIDFunc(id)),
 				test.path,
 			)
 			require.Error(t, err)
@@ -404,7 +424,7 @@ func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem_Errors() {
 // TestBackupAndRestoreSingleItem_Errors2 exercises some edge cases in the
 // package-private restoreSingleItem function. It helps ensure kopia behaves the
 // way we expect.
-func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem_Errors2() {
+func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem_Errors2() {
 	table := []struct {
 		name        string
 		rootDirFunc func(*testing.T, context.Context, *KopiaWrapper) fs.Entry
@@ -428,19 +448,9 @@ func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem_Errors2() {
 
 	for _, test := range table {
 		suite.T().Run(test.name, func(t *testing.T) {
-			ctx := context.Background()
+			_, err := suite.k.restoreSingleItem(
-
+				suite.ctx,
-			k, err := openKopiaRepo(t, ctx)
+				test.rootDirFunc(t, suite.ctx, suite.k),
-			require.NoError(t, err)
-			defer func() {
-				assert.NoError(t, k.Close(ctx))
-			}()
-
-			setupSimpleRepo(t, ctx, k)
-
-			_, err = k.restoreSingleItem(
-				ctx,
-				test.rootDirFunc(t, ctx, k),
 				test.path,
 			)
 			require.Error(t, err)

src/internal/kopia/model_store_test.go

@@ -32,6 +32,8 @@ func getModelStore(t *testing.T, ctx context.Context) *ModelStore {
 // ---------------
 type ModelStoreIntegrationSuite struct {
 	suite.Suite
+	ctx context.Context
+	m   *ModelStore
 }
 
 func TestModelStoreIntegrationSuite(t *testing.T) {
@@ -50,6 +52,15 @@ func (suite *ModelStoreIntegrationSuite) SetupSuite() {
 	require.NoError(suite.T(), err)
 }
 
+func (suite *ModelStoreIntegrationSuite) SetupTest() {
+	suite.ctx = context.Background()
+	suite.m = getModelStore(suite.T(), suite.ctx)
+}
+
+func (suite *ModelStoreIntegrationSuite) TearDownTest() {
+	assert.NoError(suite.T(), suite.m.wrapper.Close(suite.ctx))
+}
+
 func (suite *ModelStoreIntegrationSuite) TestBadTagsErrors() {
 	table := []struct {
 		name string
@@ -69,38 +80,24 @@ func (suite *ModelStoreIntegrationSuite) TestBadTagsErrors() {
 		},
 	}
 
-	ctx := context.Background()
-	t := suite.T()
-
-	m := getModelStore(t, ctx)
-	defer func() {
-		assert.NoError(t, m.wrapper.Close(ctx))
-	}()
-
 	for _, test := range table {
 		suite.T().Run(test.name, func(t *testing.T) {
 			foo := &fooModel{Bar: uuid.NewString()}
 			foo.Tags = test.tags
 
-			assert.Error(t, m.Put(ctx, BackupOpModel, foo))
+			assert.Error(t, suite.m.Put(suite.ctx, BackupOpModel, foo))
-			assert.Error(t, m.Update(ctx, BackupOpModel, foo))
+			assert.Error(t, suite.m.Update(suite.ctx, BackupOpModel, foo))
 
-			_, err := m.GetIDsForType(ctx, BackupOpModel, test.tags)
+			_, err := suite.m.GetIDsForType(suite.ctx, BackupOpModel, test.tags)
 			assert.Error(t, err)
 		})
 	}
 }
 
 func (suite *ModelStoreIntegrationSuite) TestNoIDsErrors() {
-	ctx := context.Background()
 	t := suite.T()
 	theModelType := BackupOpModel
 
-	m := getModelStore(t, ctx)
-	defer func() {
-		assert.NoError(t, m.wrapper.Close(ctx))
-	}()
-
 	noStableID := &fooModel{Bar: uuid.NewString()}
 	noStableID.StableID = ""
 	noStableID.ModelStoreID = manifest.ID(uuid.NewString())
@@ -109,54 +106,43 @@ func (suite *ModelStoreIntegrationSuite) TestNoIDsErrors() {
 	noModelStoreID.StableID = model.ID(uuid.NewString())
 	noModelStoreID.ModelStoreID = ""
 
-	assert.Error(t, m.Update(ctx, theModelType, noStableID))
+	assert.Error(t, suite.m.Update(suite.ctx, theModelType, noStableID))
-	assert.Error(t, m.Update(ctx, theModelType, noModelStoreID))
+	assert.Error(t, suite.m.Update(suite.ctx, theModelType, noModelStoreID))
 
-	assert.Error(t, m.Get(ctx, theModelType, "", nil))
+	assert.Error(t, suite.m.Get(suite.ctx, theModelType, "", nil))
-	assert.Error(t, m.GetWithModelStoreID(ctx, theModelType, "", nil))
+	assert.Error(t, suite.m.GetWithModelStoreID(suite.ctx, theModelType, "", nil))
 
-	assert.Error(t, m.Delete(ctx, theModelType, ""))
+	assert.Error(t, suite.m.Delete(suite.ctx, theModelType, ""))
-	assert.Error(t, m.DeleteWithModelStoreID(ctx, ""))
+	assert.Error(t, suite.m.DeleteWithModelStoreID(suite.ctx, ""))
 }
 
 func (suite *ModelStoreIntegrationSuite) TestBadModelTypeErrors() {
-	ctx := context.Background()
 	t := suite.T()
 
-	m := getModelStore(t, ctx)
-	defer func() {
-		assert.NoError(t, m.wrapper.Close(ctx))
-	}()
-
 	foo := &fooModel{Bar: uuid.NewString()}
 
-	assert.Error(t, m.Put(ctx, UnknownModel, foo))
+	assert.Error(t, suite.m.Put(suite.ctx, UnknownModel, foo))
 
-	require.NoError(t, m.Put(ctx, BackupOpModel, foo))
+	require.NoError(t, suite.m.Put(suite.ctx, BackupOpModel, foo))
 
-	_, err := m.GetIDsForType(ctx, UnknownModel, nil)
+	_, err := suite.m.GetIDsForType(suite.ctx, UnknownModel, nil)
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "model type")
 }
 
 func (suite *ModelStoreIntegrationSuite) TestBadTypeErrors() {
-	ctx := context.Background()
 	t := suite.T()
 
-	m := getModelStore(t, ctx)
-	defer func() {
-		assert.NoError(t, m.wrapper.Close(ctx))
-	}()
-
 	foo := &fooModel{Bar: uuid.NewString()}
 
-	require.NoError(t, m.Put(ctx, BackupOpModel, foo))
+	require.NoError(t, suite.m.Put(suite.ctx, BackupOpModel, foo))
 
 	returned := &fooModel{}
-	assert.Error(t, m.Get(ctx, RestoreOpModel, foo.StableID, returned))
+	assert.Error(t, suite.m.Get(suite.ctx, RestoreOpModel, foo.StableID, returned))
-	assert.Error(t, m.GetWithModelStoreID(ctx, RestoreOpModel, foo.ModelStoreID, returned))
+	assert.Error(
+		t, suite.m.GetWithModelStoreID(suite.ctx, RestoreOpModel, foo.ModelStoreID, returned))
 
-	assert.Error(t, m.Delete(ctx, RestoreOpModel, foo.StableID))
+	assert.Error(t, suite.m.Delete(suite.ctx, RestoreOpModel, foo.StableID))
 }
 
 func (suite *ModelStoreIntegrationSuite) TestPutGet() {
@@ -187,21 +173,13 @@ func (suite *ModelStoreIntegrationSuite) TestPutGet() {
 		},
 	}
 
-	ctx := context.Background()
-	t := suite.T()
-
-	m := getModelStore(t, ctx)
-	defer func() {
-		assert.NoError(t, m.wrapper.Close(ctx))
-	}()
-
 	for _, test := range table {
 		suite.T().Run(test.t.String(), func(t *testing.T) {
 			foo := &fooModel{Bar: uuid.NewString()}
 			// Avoid some silly test errors from comparing nil to empty map.
 			foo.Tags = map[string]string{}
 
-			err := m.Put(ctx, test.t, foo)
+			err := suite.m.Put(suite.ctx, test.t, foo)
 			test.check(t, err)
 
 			if test.hasErr {
@@ -212,11 +190,11 @@ func (suite *ModelStoreIntegrationSuite) TestPutGet() {
 			require.NotEmpty(t, foo.StableID)
 
 			returned := &fooModel{}
-			err = m.Get(ctx, test.t, foo.StableID, returned)
+			err = suite.m.Get(suite.ctx, test.t, foo.StableID, returned)
 			require.NoError(t, err)
 			assert.Equal(t, foo, returned)
 
-			err = m.GetWithModelStoreID(ctx, test.t, foo.ModelStoreID, returned)
+			err = suite.m.GetWithModelStoreID(suite.ctx, test.t, foo.ModelStoreID, returned)
 			require.NoError(t, err)
 			assert.Equal(t, foo, returned)
 		})
@@ -224,46 +202,82 @@ func (suite *ModelStoreIntegrationSuite) TestPutGet() {
 }
 
 func (suite *ModelStoreIntegrationSuite) TestPutGet_WithTags() {
-	ctx := context.Background()
 	t := suite.T()
 	theModelType := BackupOpModel
 
-	m := getModelStore(t, ctx)
-	defer func() {
-		assert.NoError(t, m.wrapper.Close(ctx))
-	}()
-
 	foo := &fooModel{Bar: uuid.NewString()}
 	foo.Tags = map[string]string{
 		"bar": "baz",
 	}
 
-	require.NoError(t, m.Put(ctx, theModelType, foo))
+	require.NoError(t, suite.m.Put(suite.ctx, theModelType, foo))
 
 	require.NotEmpty(t, foo.ModelStoreID)
 	require.NotEmpty(t, foo.StableID)
 
 	returned := &fooModel{}
-	err := m.Get(ctx, theModelType, foo.StableID, returned)
+	err := suite.m.Get(suite.ctx, theModelType, foo.StableID, returned)
 	require.NoError(t, err)
 	assert.Equal(t, foo, returned)
 
-	err = m.GetWithModelStoreID(ctx, theModelType, foo.ModelStoreID, returned)
+	err = suite.m.GetWithModelStoreID(suite.ctx, theModelType, foo.ModelStoreID, returned)
 	require.NoError(t, err)
 	assert.Equal(t, foo, returned)
 }
 
 func (suite *ModelStoreIntegrationSuite) TestGet_NotFoundErrors() {
-	ctx := context.Background()
 	t := suite.T()
 
-	m := getModelStore(t, ctx)
+	assert.ErrorIs(t, suite.m.Get(suite.ctx, BackupOpModel, "baz", nil), manifest.ErrNotFound)
-	defer func() {
+	assert.ErrorIs(
-		assert.NoError(t, m.wrapper.Close(ctx))
+		t, suite.m.GetWithModelStoreID(suite.ctx, BackupOpModel, "baz", nil), manifest.ErrNotFound)
-	}()
+}
 
-	assert.ErrorIs(t, m.Get(ctx, BackupOpModel, "baz", nil), manifest.ErrNotFound)
+func (suite *ModelStoreIntegrationSuite) TestPutGetOfType() {
-	assert.ErrorIs(t, m.GetWithModelStoreID(ctx, BackupOpModel, "baz", nil), manifest.ErrNotFound)
+	table := []struct {
+		t      modelType
+		check  require.ErrorAssertionFunc
+		hasErr bool
+	}{
+		{
+			t:      UnknownModel,
+			check:  require.Error,
+			hasErr: true,
+		},
+		{
+			t:      BackupOpModel,
+			check:  require.NoError,
+			hasErr: false,
+		},
+		{
+			t:      RestoreOpModel,
+			check:  require.NoError,
+			hasErr: false,
+		},
+		{
+			t:      RestorePointModel,
+			check:  require.NoError,
+			hasErr: false,
+		},
+	}
+
+	for _, test := range table {
+		suite.T().Run(test.t.String(), func(t *testing.T) {
+			foo := &fooModel{Bar: uuid.NewString()}
+
+			err := suite.m.Put(suite.ctx, test.t, foo)
+			test.check(t, err)
+
+			if test.hasErr {
+				return
+			}
+
+			ids, err := suite.m.GetIDsForType(suite.ctx, test.t, nil)
+			require.NoError(t, err)
+
+			assert.Len(t, ids, 1)
+		})
+	}
 }
 
 func (suite *ModelStoreIntegrationSuite) TestPutUpdate() {
@@ -333,61 +347,6 @@ func (suite *ModelStoreIntegrationSuite) TestPutUpdate() {
 	}
 }
 
-func (suite *ModelStoreIntegrationSuite) TestPutGetOfType() {
-	table := []struct {
-		t      modelType
-		check  require.ErrorAssertionFunc
-		hasErr bool
-	}{
-		{
-			t:      UnknownModel,
-			check:  require.Error,
-			hasErr: true,
-		},
-		{
-			t:      BackupOpModel,
-			check:  require.NoError,
-			hasErr: false,
-		},
-		{
-			t:      RestoreOpModel,
-			check:  require.NoError,
-			hasErr: false,
-		},
-		{
-			t:      RestorePointModel,
-			check:  require.NoError,
-			hasErr: false,
-		},
-	}
-
-	ctx := context.Background()
-	t := suite.T()
-
-	m := getModelStore(t, ctx)
-	defer func() {
-		assert.NoError(t, m.wrapper.Close(ctx))
-	}()
-
-	for _, test := range table {
-		suite.T().Run(test.t.String(), func(t *testing.T) {
-			foo := &fooModel{Bar: uuid.NewString()}
-
-			err := m.Put(ctx, test.t, foo)
-			test.check(t, err)
-
-			if test.hasErr {
-				return
-			}
-
-			ids, err := m.GetIDsForType(ctx, test.t, nil)
-			require.NoError(t, err)
-
-			assert.Len(t, ids, 1)
-		})
-	}
-}
-
 func (suite *ModelStoreIntegrationSuite) TestPutUpdate_FailsNotMatchingPrev() {
 	startModelType := BackupOpModel
 
@@ -432,37 +391,25 @@ func (suite *ModelStoreIntegrationSuite) TestPutUpdate_FailsNotMatchingPrev() {
 }
 
 func (suite *ModelStoreIntegrationSuite) TestPutDelete() {
-	ctx := context.Background()
 	t := suite.T()
 	theModelType := BackupOpModel
 
-	m := getModelStore(t, ctx)
-	defer func() {
-		assert.NoError(t, m.wrapper.Close(ctx))
-	}()
-
 	foo := &fooModel{Bar: uuid.NewString()}
 
-	require.NoError(t, m.Put(ctx, theModelType, foo))
+	require.NoError(t, suite.m.Put(suite.ctx, theModelType, foo))
 
-	require.NoError(t, m.Delete(ctx, theModelType, foo.StableID))
+	require.NoError(t, suite.m.Delete(suite.ctx, theModelType, foo.StableID))
 
 	returned := &fooModel{}
-	err := m.GetWithModelStoreID(ctx, theModelType, foo.ModelStoreID, returned)
+	err := suite.m.GetWithModelStoreID(suite.ctx, theModelType, foo.ModelStoreID, returned)
 	assert.ErrorIs(t, err, manifest.ErrNotFound)
 }
 
 func (suite *ModelStoreIntegrationSuite) TestPutDelete_BadIDsNoop() {
-	ctx := context.Background()
 	t := suite.T()
 
-	m := getModelStore(t, ctx)
+	assert.NoError(t, suite.m.Delete(suite.ctx, BackupOpModel, "foo"))
-	defer func() {
+	assert.NoError(t, suite.m.DeleteWithModelStoreID(suite.ctx, "foo"))
-		assert.NoError(t, m.wrapper.Close(ctx))
-	}()
-
-	assert.NoError(t, m.Delete(ctx, BackupOpModel, "foo"))
-	assert.NoError(t, m.DeleteWithModelStoreID(ctx, "foo"))
 }
 
 // ---------------