From 944887268d7b30a376585b262f3362f5e2cbfb62 Mon Sep 17 00:00:00 2001 From: neha-Gupta1 Date: Mon, 15 May 2023 12:40:01 +0530 Subject: [PATCH] remove testing object --- src/cmd/factory/impl/common.go | 16 +- .../graph_connector_onedrive_test.go | 50 ++++- .../graph_connector_onedrive_test_helper.go | 191 ++++++++++++------ .../connector/graph_connector_test.go | 41 +++- .../connector/graph_connector_test_helper.go | 42 ++-- 5 files changed, 229 insertions(+), 111 deletions(-) diff --git a/src/cmd/factory/impl/common.go b/src/cmd/factory/impl/common.go index c899e9b21..d5167d9a5 100644 --- a/src/cmd/factory/impl/common.go +++ b/src/cmd/factory/impl/common.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "strings" - "testing" "time" "github.com/alcionai/clues" @@ -397,9 +396,10 @@ func generateAndRestoreOnedriveItems( cols = append(cols, col...) } - // TODO Neha: work on this - t := testing.T{} - input := connector.DataForInfo(&t, service, cols, version.Backup) + input, err := connector.DataForInfo(service, cols, version.Backup) + if err != nil { + return nil, err + } // collections := getCollections( // service, @@ -423,8 +423,8 @@ func generateAndRestoreOnedriveItems( Dest: tester.DefaultTestRestoreDestination(""), } - _, _, collections, _ := connector.GetCollectionsAndExpected( - &t, + _, _, collections, _, err := connector.GetCollectionsAndExpected( + // &t, config, // service, // tenantID, @@ -432,6 +432,10 @@ func generateAndRestoreOnedriveItems( input, version.Backup) + if err != nil { + return nil, err + } + return gc.ConsumeRestoreCollections(ctx, version.Backup, acct, sel, dest, opts, collections, errs) } diff --git a/src/internal/connector/graph_connector_onedrive_test.go b/src/internal/connector/graph_connector_onedrive_test.go index b8fbaa0a1..7e459e817 100644 --- a/src/internal/connector/graph_connector_onedrive_test.go +++ b/src/internal/connector/graph_connector_onedrive_test.go @@ -449,12 +449,18 @@ func testRestoreAndBackupMultipleFilesAndFoldersNoPermissions( }, } - expected := DataForInfo(suite.T(), suite.BackupService(), cols, version.Backup) + expected, err := DataForInfo(suite.BackupService(), cols, version.Backup) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } for vn := startVersion; vn <= version.Backup; vn++ { suite.Run(fmt.Sprintf("Version%d", vn), func() { t := suite.T() - input := DataForInfo(t, suite.BackupService(), cols, vn) + input, err := DataForInfo(suite.BackupService(), cols, vn) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } testData := restoreBackupInfoMultiVersion{ service: suite.BackupService(), @@ -658,7 +664,10 @@ func testPermissionsRestoreAndBackup(suite oneDriveSuite, startVersion int) { }, } - expected := DataForInfo(suite.T(), suite.BackupService(), cols, version.Backup) + expected, err := DataForInfo(suite.BackupService(), cols, version.Backup) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } for vn := startVersion; vn <= version.Backup; vn++ { suite.Run(fmt.Sprintf("Version%d", vn), func() { @@ -666,7 +675,10 @@ func testPermissionsRestoreAndBackup(suite oneDriveSuite, startVersion int) { // Ideally this can always be true or false and still // work, but limiting older versions to use emails so as // to validate that flow as well. - input := DataForInfo(t, suite.BackupService(), cols, vn) + input, err := DataForInfo(suite.BackupService(), cols, vn) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } testData := restoreBackupInfoMultiVersion{ service: suite.BackupService(), @@ -742,12 +754,18 @@ func testPermissionsBackupAndNoRestore(suite oneDriveSuite, startVersion int) { }, } - expected := DataForInfo(suite.T(), suite.BackupService(), expectedCols, version.Backup) + expected, err := DataForInfo(suite.BackupService(), expectedCols, version.Backup) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } for vn := startVersion; vn <= version.Backup; vn++ { suite.Run(fmt.Sprintf("Version%d", vn), func() { t := suite.T() - input := DataForInfo(t, suite.BackupService(), inputCols, vn) + input, err := DataForInfo(suite.BackupService(), inputCols, vn) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } testData := restoreBackupInfoMultiVersion{ service: suite.BackupService(), @@ -918,7 +936,10 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio }, } - expected := DataForInfo(suite.T(), suite.BackupService(), cols, version.Backup) + expected, err := DataForInfo(suite.BackupService(), cols, version.Backup) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } for vn := startVersion; vn <= version.Backup; vn++ { suite.Run(fmt.Sprintf("Version%d", vn), func() { @@ -926,7 +947,10 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio // Ideally this can always be true or false and still // work, but limiting older versions to use emails so as // to validate that flow as well. - input := DataForInfo(t, suite.BackupService(), cols, vn) + input, err := DataForInfo(suite.BackupService(), cols, vn) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } testData := restoreBackupInfoMultiVersion{ service: suite.BackupService(), @@ -1032,12 +1056,18 @@ func testRestoreFolderNamedFolderRegression( }, } - expected := DataForInfo(suite.T(), suite.BackupService(), cols, version.Backup) + expected, err := DataForInfo(suite.BackupService(), cols, version.Backup) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } for vn := startVersion; vn <= version.Backup; vn++ { suite.Run(fmt.Sprintf("Version%d", vn), func() { t := suite.T() - input := DataForInfo(t, suite.BackupService(), cols, vn) + input, err := DataForInfo(suite.BackupService(), cols, vn) + if err != nil { + assert.FailNow(suite.T(), err.Error()) + } testData := restoreBackupInfoMultiVersion{ service: suite.BackupService(), diff --git a/src/internal/connector/graph_connector_onedrive_test_helper.go b/src/internal/connector/graph_connector_onedrive_test_helper.go index 521851fd2..24a24311d 100644 --- a/src/internal/connector/graph_connector_onedrive_test_helper.go +++ b/src/internal/connector/graph_connector_onedrive_test_helper.go @@ -2,12 +2,10 @@ package connector import ( "encoding/json" - "testing" + "fmt" "github.com/alcionai/clues" "github.com/google/uuid" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "golang.org/x/exp/maps" odConsts "github.com/alcionai/corso/src/internal/connector/onedrive/consts" @@ -76,7 +74,7 @@ type onedriveCollection struct { items []ItemInfo aux []ItemInfo backupVersion int - t *testing.T + // t *testing.T } func (c onedriveCollection) collection() ColInfo { @@ -94,7 +92,7 @@ func (c onedriveCollection) collection() ColInfo { } func NewOneDriveCollection( - t *testing.T, + // t *testing.T, service path.ServiceType, PathElements []string, backupVersion int, @@ -103,123 +101,170 @@ func NewOneDriveCollection( service: service, PathElements: PathElements, backupVersion: backupVersion, - t: t, + // t: t, } } func DataForInfo( - t *testing.T, + // t *testing.T, service path.ServiceType, cols []OnedriveColInfo, backupVersion int, -) []ColInfo { - var res []ColInfo +) ([]ColInfo, error) { + var ( + res []ColInfo + err error + ) for _, c := range cols { - onedriveCol := NewOneDriveCollection(t, service, c.PathElements, backupVersion) + onedriveCol := NewOneDriveCollection(service, c.PathElements, backupVersion) for _, f := range c.Files { - onedriveCol.withFile(f.Name, f.Data, f.Perms) + _, err = onedriveCol.withFile(f.Name, f.Data, f.Perms) + if err != nil { + return res, err + } } for _, d := range c.Folders { - onedriveCol.withFolder(d.Name, d.Perms) + _, err = onedriveCol.withFolder(d.Name, d.Perms) + if err != nil { + return res, err + } } - onedriveCol.withPermissions(c.Perms) + _, err = onedriveCol.withPermissions(c.Perms) + if err != nil { + return res, err + } res = append(res, onedriveCol.collection()) } - return res + return res, nil } -func (c *onedriveCollection) withFile(name string, fileData []byte, perm PermData) *onedriveCollection { +func (c *onedriveCollection) withFile(name string, fileData []byte, perm PermData) (*onedriveCollection, error) { switch c.backupVersion { case 0: // Lookups will occur using the most recent version of things so we need // the embedded file name to match that. - c.items = append(c.items, onedriveItemWithData( - c.t, + item, err := onedriveItemWithData( + // c.t, name, name+metadata.DataFileSuffix, - fileData)) + fileData) + + if err != nil { + return c, err + } + + c.items = append(c.items, item) // v1-5, early metadata design case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker, version.OneDrive4DirIncludesPermissions, version.OneDrive5DirMetaNoName: - c.items = append(c.items, onedriveItemWithData( - c.t, - name+metadata.DataFileSuffix, - name+metadata.DataFileSuffix, - fileData)) - md := onedriveMetadata( - c.t, + items, err := onedriveItemWithData( + // c.t, + name+metadata.DataFileSuffix, + name+metadata.DataFileSuffix, + fileData) + + if err != nil { + return c, err + } + + c.items = append(c.items, items) + + md, err := onedriveMetadata( + // c.t, "", name+metadata.MetaFileSuffix, name+metadata.MetaFileSuffix, perm, c.backupVersion >= versionPermissionSwitchedToID) + + if err != nil { + return c, err + } + c.items = append(c.items, md) c.aux = append(c.aux, md) // v6+ current metadata design case version.OneDrive6NameInMeta, version.OneDrive7LocationRef, version.All8MigrateUserPNToID: - c.items = append(c.items, onedriveItemWithData( - c.t, + item, err := onedriveItemWithData( + // c.t, name+metadata.DataFileSuffix, name+metadata.DataFileSuffix, - fileData)) + fileData) - md := onedriveMetadata( - c.t, + if err != nil { + return c, err + } + + c.items = append(c.items, item) + + md, err := onedriveMetadata( + // c.t, name, name+metadata.MetaFileSuffix, name, perm, c.backupVersion >= versionPermissionSwitchedToID) + + if err != nil { + return c, err + } + c.items = append(c.items, md) c.aux = append(c.aux, md) default: - assert.FailNowf(c.t, "bad backup version", "version %d", c.backupVersion) + return c, clues.New(fmt.Sprintf("bad backup version. version %d", c.backupVersion)) + // assert.FailNowf(c.t, "bad backup version", "version %d", c.backupVersion) } - return c + return c, nil } -func (c *onedriveCollection) withFolder(name string, perm PermData) *onedriveCollection { +func (c *onedriveCollection) withFolder(name string, perm PermData) (*onedriveCollection, error) { switch c.backupVersion { case 0, version.OneDrive4DirIncludesPermissions, version.OneDrive5DirMetaNoName, version.OneDrive6NameInMeta, version.OneDrive7LocationRef, version.All8MigrateUserPNToID: - return c + return c, nil case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker: - c.items = append( - c.items, - onedriveMetadata( - c.t, - "", - name+metadata.DirMetaFileSuffix, - name+metadata.DirMetaFileSuffix, - perm, - c.backupVersion >= versionPermissionSwitchedToID)) + item, err := onedriveMetadata( + // c.t, + "", + name+metadata.DirMetaFileSuffix, + name+metadata.DirMetaFileSuffix, + perm, + c.backupVersion >= versionPermissionSwitchedToID) + + c.items = append(c.items, item) + + if err != nil { + return c, err + } default: - assert.FailNowf(c.t, "bad backup version", "version %d", c.backupVersion) + return c, clues.New(fmt.Sprintf("bad backup version.version %d", c.backupVersion)) + // assert.FailNowf(c.t, "bad backup version", "version %d", c.backupVersion) } - return c + return c, nil } // withPermissions adds permissions to the folder represented by this // onedriveCollection. -func (c *onedriveCollection) withPermissions(perm PermData) *onedriveCollection { +func (c *onedriveCollection) withPermissions(perm PermData) (*onedriveCollection, error) { // These versions didn't store permissions for the folder or didn't store them // in the folder's collection. if c.backupVersion < version.OneDrive4DirIncludesPermissions { - return c + return c, nil } name := c.PathElements[len(c.PathElements)-1] @@ -231,21 +276,24 @@ func (c *onedriveCollection) withPermissions(perm PermData) *onedriveCollection } if name == odConsts.RootPathDir { - return c + return c, nil } - md := onedriveMetadata( - c.t, + md, err := onedriveMetadata( + // c.t, name, metaName+metadata.DirMetaFileSuffix, metaName+metadata.DirMetaFileSuffix, perm, c.backupVersion >= versionPermissionSwitchedToID) + if err != nil { + return c, err + } c.items = append(c.items, md) c.aux = append(c.aux, md) - return c + return c, err } type testOneDriveData struct { @@ -254,11 +302,11 @@ type testOneDriveData struct { } func onedriveItemWithData( - t *testing.T, + // t *testing.T, name, lookupKey string, fileData []byte, -) ItemInfo { - t.Helper() +) (ItemInfo, error) { + // t.Helper() content := testOneDriveData{ FileName: lookupKey, @@ -266,42 +314,48 @@ func onedriveItemWithData( } serialized, err := json.Marshal(content) - require.NoError(t, err, clues.ToCore(err)) + if err != nil { + return ItemInfo{}, clues.Stack(err) + } + // require.NoError(t, err, clues.ToCore(err)) return ItemInfo{ name: name, data: serialized, lookupKey: lookupKey, - } + }, nil } func onedriveMetadata( - t *testing.T, + // t *testing.T, fileName, itemID, lookupKey string, perm PermData, permUseID bool, -) ItemInfo { - t.Helper() +) (ItemInfo, error) { + // t.Helper() testMeta := getMetadata(fileName, perm, permUseID) testMetaJSON, err := json.Marshal(testMeta) - require.NoError(t, err, "marshalling metadata", clues.ToCore(err)) + if err != nil { + return ItemInfo{}, clues.Wrap(err, "marshalling metadata") + } + // require.NoError(t, err, "marshalling metadata", clues.ToCore(err)) return ItemInfo{ name: itemID, data: testMetaJSON, lookupKey: lookupKey, - } + }, nil } func GetCollectionsAndExpected( - t *testing.T, + // t *testing.T, config ConfigInfo, testCollections []ColInfo, backupVersion int, -) (int, int, []data.RestoreCollection, map[string]map[string][]byte) { - t.Helper() +) (int, int, []data.RestoreCollection, map[string]map[string][]byte, error) { + // t.Helper() var ( collections []data.RestoreCollection @@ -311,8 +365,8 @@ func GetCollectionsAndExpected( ) for _, owner := range config.ResourceOwners { - numItems, kopiaItems, ownerCollections, userExpectedData := collectionsForInfo( - t, + numItems, kopiaItems, ownerCollections, userExpectedData, err := collectionsForInfo( + // t, config.Service, config.Tenant, owner, @@ -320,6 +374,9 @@ func GetCollectionsAndExpected( testCollections, backupVersion, ) + if err != nil { + return totalItems, totalKopiaItems, collections, expectedData, err + } collections = append(collections, ownerCollections...) totalItems += numItems @@ -328,5 +385,5 @@ func GetCollectionsAndExpected( maps.Copy(expectedData, userExpectedData) } - return totalItems, totalKopiaItems, collections, expectedData + return totalItems, totalKopiaItems, collections, expectedData, nil } diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 62840153d..73b8cdff3 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -545,12 +545,16 @@ func runRestoreBackupTest( Dest: tester.DefaultTestRestoreDestination(""), } - totalItems, totalKopiaItems, collections, expectedData := GetCollectionsAndExpected( - t, + totalItems, totalKopiaItems, collections, expectedData, err := GetCollectionsAndExpected( + // t, config, test.collections, version.Backup) + if err != nil { + assert.FailNow(t, "failed with error", err) + } + runRestore( t, ctx, @@ -591,12 +595,16 @@ func runRestoreTestWithVerion( Dest: tester.DefaultTestRestoreDestination(""), } - totalItems, _, collections, _ := GetCollectionsAndExpected( - t, + totalItems, _, collections, _, err := GetCollectionsAndExpected( + // t, config, test.collectionsPrevious, test.backupVersion) + if err != nil { + assert.FailNow(t, "failed with error", err) + } + runRestore( t, ctx, @@ -630,12 +638,16 @@ func runRestoreBackupTestVersions( Dest: tester.DefaultTestRestoreDestination(""), } - totalItems, _, collections, _ := GetCollectionsAndExpected( - t, + totalItems, _, collections, _, err := GetCollectionsAndExpected( + // t, config, test.collectionsPrevious, test.backupVersion) + if err != nil { + assert.FailNow(t, "failed with error", err) + } + runRestore( t, ctx, @@ -645,12 +657,16 @@ func runRestoreBackupTestVersions( totalItems) // Get expected output for new version. - totalItems, totalKopiaItems, _, expectedData := GetCollectionsAndExpected( - t, + totalItems, totalKopiaItems, _, expectedData, err := GetCollectionsAndExpected( + // t, config, test.collectionsLatest, version.Backup) + if err != nil { + assert.FailNow(t, "failed with error", err) + } + runBackupAndCompare( t, ctx, @@ -1014,8 +1030,8 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames }, }) - totalItems, _, collections, expectedData := collectionsForInfo( - t, + totalItems, _, collections, expectedData, err := collectionsForInfo( + // t, test.service, suite.connector.tenant, suite.user, @@ -1023,6 +1039,11 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames []ColInfo{collection}, version.Backup, ) + + if err != nil { + assert.FailNow(t, "failed with error", err) + } + allItems += totalItems for k, v := range expectedData { diff --git a/src/internal/connector/graph_connector_test_helper.go b/src/internal/connector/graph_connector_test_helper.go index ba3c8c5c6..cdd383e97 100644 --- a/src/internal/connector/graph_connector_test_helper.go +++ b/src/internal/connector/graph_connector_test_helper.go @@ -4,10 +4,6 @@ import ( "bytes" "context" "io" - "testing" - - "github.com/alcionai/clues" - "github.com/stretchr/testify/require" exchMock "github.com/alcionai/corso/src/internal/connector/exchange/mock" "github.com/alcionai/corso/src/internal/connector/onedrive/metadata" @@ -53,17 +49,20 @@ type ConfigInfo struct { } func mustToDataLayerPath( - t *testing.T, + // t *testing.T, service path.ServiceType, tenant, resourceOwner string, category path.CategoryType, elements []string, isItem bool, -) path.Path { +) (path.Path, error) { res, err := path.Build(tenant, resourceOwner, service, category, isItem, elements...) - require.NoError(t, err, clues.ToCore(err)) + if err != nil { + return nil, err + } + // require.NoError(t, err, clues.ToCore(err)) - return res + return res, err } // backupOutputPathFromRestore returns a path.Path denoting the location in @@ -71,10 +70,10 @@ func mustToDataLayerPath( // combination of the location the data was recently restored to and where the // data was originally in the hierarchy. func backupOutputPathFromRestore( - t *testing.T, + // t *testing.T, restoreDest control.RestoreDestination, inputPath path.Path, -) path.Path { +) (path.Path, error) { base := []string{restoreDest.ContainerName} // OneDrive has leading information like the drive ID. @@ -91,8 +90,8 @@ func backupOutputPathFromRestore( base = append(base, inputPath.Folders()...) } - return mustToDataLayerPath( - t, + path, err := mustToDataLayerPath( + // t, inputPath.Service(), inputPath.Tenant(), inputPath.ResourceOwner(), @@ -100,6 +99,7 @@ func backupOutputPathFromRestore( base, false, ) + return path, err } // TODO(ashmrtn): Make this an actual mock class that can be used in other @@ -122,13 +122,13 @@ func (rc mockRestoreCollection) Fetch( } func collectionsForInfo( - t *testing.T, + // t *testing.T, service path.ServiceType, tenant, user string, dest control.RestoreDestination, allInfo []ColInfo, backupVersion int, -) (int, int, []data.RestoreCollection, map[string]map[string][]byte) { +) (int, int, []data.RestoreCollection, map[string]map[string][]byte, error) { var ( collections = make([]data.RestoreCollection, 0, len(allInfo)) expectedData = make(map[string]map[string][]byte, len(allInfo)) @@ -137,17 +137,23 @@ func collectionsForInfo( ) for _, info := range allInfo { - pth := mustToDataLayerPath( - t, + pth, err := mustToDataLayerPath( + // t, service, tenant, user, info.Category, info.PathElements, false) + if err != nil { + return totalItems, kopiaEntries, collections, expectedData, err + } mc := exchMock.NewCollection(pth, pth, len(info.Items)) - baseDestPath := backupOutputPathFromRestore(t, dest, pth) + baseDestPath, err := backupOutputPathFromRestore(dest, pth) + if err != nil { + return totalItems, kopiaEntries, collections, expectedData, err + } baseExpected := expectedData[baseDestPath.String()] if baseExpected == nil { @@ -184,5 +190,5 @@ func collectionsForInfo( kopiaEntries += len(info.Items) } - return totalItems, kopiaEntries, collections, expectedData + return totalItems, kopiaEntries, collections, expectedData, nil }