From 9fe9cd1ce61f3d0730b7eb602c04f181324b7719 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Thu, 17 Nov 2022 15:03:31 -0800 Subject: [PATCH] Use prefix matching in GraphConnector tests (#1548) ## Description Switch to using prefix matchers for restore+backup tests as selectors now support them by default. Not using prefix matchers could eventually lead to failed tests as some tests may have subfolders that could lead to repeated items depending on how the selector is constructed. For example, a selector with scopes matching on `["inbox", "inbox/Work"]` could lead to matching `inbox/Work` twice if only prefix matching is done and GraphConnector iterates over all scopes ## Type of change - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [x] :robot: Test - [ ] :computer: CI/Deployment - [ ] :hamster: Trivial/Minor ## Issue(s) * #913 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- .../connector/graph_connector_helper_test.go | 103 ++++++++---------- .../connector/graph_connector_test.go | 28 ++++- 2 files changed, 66 insertions(+), 65 deletions(-) diff --git a/src/internal/connector/graph_connector_helper_test.go b/src/internal/connector/graph_connector_helper_test.go index 1228941e6..11a6411b3 100644 --- a/src/internal/connector/graph_connector_helper_test.go +++ b/src/internal/connector/graph_connector_helper_test.go @@ -653,11 +653,12 @@ func checkCollections( expected map[string]map[string][]byte, got []data.Collection, ) { - checkHasCollections(t, expected, got) + collectionsWithItems := []data.Collection{} gotItems := 0 for _, returned := range got { + startingItems := gotItems service := returned.FullPath().Service() category := returned.FullPath().Category() expectedColData := expected[returned.FullPath().String()] @@ -674,44 +675,47 @@ func checkCollections( compareItem(t, expectedColData, service, category, item) } + + if gotItems != startingItems { + collectionsWithItems = append(collectionsWithItems, returned) + } } assert.Equal(t, expectedItems, gotItems, "expected items") + checkHasCollections(t, expected, collectionsWithItems) } -func mustParsePath(t *testing.T, p string, isItem bool) path.Path { - res, err := path.FromDataLayerPath(p, isItem) - require.NoError(t, err) - - return res +type destAndCats struct { + resourceOwner string + dest string + cats map[path.CategoryType]struct{} } func makeExchangeBackupSel( t *testing.T, - expected map[string]map[string][]byte, + dests []destAndCats, ) selectors.Selector { sel := selectors.NewExchangeBackup() toInclude := [][]selectors.ExchangeScope{} - for p := range expected { - pth := mustParsePath(t, p, false) - require.Equal(t, path.ExchangeService.String(), pth.Service().String()) + for _, d := range dests { + for c := range d.cats { + builder := sel.MailFolders - builder := sel.MailFolders + switch c { + case path.ContactsCategory: + builder = sel.ContactFolders + case path.EventsCategory: + builder = sel.EventCalendars + case path.EmailCategory: // already set + } - switch pth.Category() { - case path.ContactsCategory: - builder = sel.ContactFolders - case path.EventsCategory: - builder = sel.EventCalendars - case path.EmailCategory: // already set + toInclude = append(toInclude, builder( + []string{d.resourceOwner}, + []string{d.dest}, + selectors.PrefixMatch(), + )) } - - toInclude = append(toInclude, builder( - []string{pth.ResourceOwner()}, - []string{backupInputFromPath(pth).String()}, - selectors.PrefixMatch(), - )) } sel.Include(toInclude...) @@ -721,18 +725,16 @@ func makeExchangeBackupSel( func makeOneDriveBackupSel( t *testing.T, - expected map[string]map[string][]byte, + dests []destAndCats, ) selectors.Selector { sel := selectors.NewOneDriveBackup() toInclude := [][]selectors.OneDriveScope{} - for p := range expected { - pth := mustParsePath(t, p, false) - require.Equal(t, path.OneDriveService.String(), pth.Service().String()) - + for _, d := range dests { toInclude = append(toInclude, sel.Folders( - []string{pth.ResourceOwner()}, - []string{backupInputFromPath(pth).String()}, + []string{d.resourceOwner}, + []string{d.dest}, + selectors.PrefixMatch(), )) } @@ -746,45 +748,26 @@ func makeOneDriveBackupSel( // multiple services are in expected. func backupSelectorForExpected( t *testing.T, - expected map[string]map[string][]byte, + service path.ServiceType, + dests []destAndCats, ) selectors.Selector { - require.NotEmpty(t, expected) + require.NotEmpty(t, dests) - // Sadly need to get the first item to figure out what sort of selector we - // need. - for p := range expected { - pth := mustParsePath(t, p, false) + switch service { + case path.ExchangeService: + return makeExchangeBackupSel(t, dests) - switch pth.Service() { - case path.ExchangeService: - return makeExchangeBackupSel(t, expected) - case path.OneDriveService: - return makeOneDriveBackupSel(t, expected) - default: - assert.FailNowf(t, "bad serivce type %s", pth.Service().String()) - } + case path.OneDriveService: + return makeOneDriveBackupSel(t, dests) + + default: + assert.FailNow(t, "unknown service type %s", service.String()) } // Fix compile error about no return. Should not reach here. return selectors.Selector{} } -// backupInputFromPath returns a path.Builder with the path that a call to Backup -// can use to backup the given items. -func backupInputFromPath( - inputPath path.Path, -) *path.Builder { - startIdx := 0 - - if inputPath.Service() == path.OneDriveService { - // OneDrive has folders that are trimmed off in the app that are present in - // Corso. - startIdx = 3 - } - - return path.Builder{}.Append(inputPath.Folders()[startIdx:]...) -} - // backupOutputPathFromRestore returns a path.Path denoting the location in // kopia the data will be placed at. The location is a data-type specific // combination of the location the data was recently restored to and where the diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index af6a6fea4..81b530bc2 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -11,7 +11,6 @@ import ( "github.com/alcionai/corso/src/internal/connector/mockconnector" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/tester" - "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -215,9 +214,22 @@ func runRestoreBackupTest( t.Logf("Restore complete in %v\n", runTime) // Run a backup and compare its output with what we put in. + cats := make(map[path.CategoryType]struct{}, len(test.collections)) + for _, c := range test.collections { + cats[c.category] = struct{}{} + } + + expectedDests := make([]destAndCats, 0, len(users)) + for _, u := range users { + expectedDests = append(expectedDests, destAndCats{ + resourceOwner: u, + dest: dest.ContainerName, + cats: cats, + }) + } backupGC := loadConnector(ctx, t) - backupSel := backupSelectorForExpected(t, expectedData) + backupSel := backupSelectorForExpected(t, test.service, expectedDests) t.Logf("Selective backup of %s\n", backupSel) start = time.Now() @@ -526,14 +538,20 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames defer flush() restoreSel := getSelectorWith(test.service) - dests := make([]control.RestoreDestination, 0, len(test.collections)) + expectedDests := make([]destAndCats, 0, len(test.collections)) allItems := 0 allExpectedData := map[string]map[string][]byte{} for i, collection := range test.collections { // Get a dest per collection so they're independent. dest := tester.DefaultTestRestoreDestination() - dests = append(dests, dest) + expectedDests = append(expectedDests, destAndCats{ + resourceOwner: suite.user, + dest: dest.ContainerName, + cats: map[path.CategoryType]struct{}{ + collection.category: {}, + }, + }) totalItems, collections, expectedData := collectionsForInfo( t, @@ -575,7 +593,7 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames // Run a backup and compare its output with what we put in. backupGC := loadConnector(ctx, t) - backupSel := backupSelectorForExpected(t, allExpectedData) + backupSel := backupSelectorForExpected(t, test.service, expectedDests) t.Log("Selective backup of", backupSel) dcs, err := backupGC.DataCollections(ctx, backupSel)