Fixup item comparisons in RestoreAndBackup tests

OneDrive poses a new problem for these tests when permissions are being
checked. Namely, we don't have the permissions for the root directory
(right now), but when we do the backup to check what was restored we
pull the permissions for the root of the subtree we backup (due to
restore as copy behavior).

This commit keeps the framework from comparing metadata on the folder
that is the root of the restored hierarchy.
This commit is contained in:
Ashlie Martinez 2023-02-21 15:31:16 -08:00
parent 4c88617783
commit d7ec24d6cd
2 changed files with 56 additions and 20 deletions

View File

@ -704,11 +704,18 @@ func compareOneDriveItem(
t *testing.T, t *testing.T,
expected map[string][]byte, expected map[string][]byte,
item data.Stream, item data.Stream,
dest control.RestoreDestination,
restorePermissions bool, restorePermissions bool,
) { ) bool {
// Skip OneDrive permissions in the folder that used to be the root. We don't
// have a good way to materialize these in the test right now.
if item.UUID() == dest.ContainerName+onedrive.DirMetaFileSuffix {
return false
}
buf, err := io.ReadAll(item.ToReader()) buf, err := io.ReadAll(item.ToReader())
if !assert.NoError(t, err) { if !assert.NoError(t, err) {
return return true
} }
name := item.UUID() name := item.UUID()
@ -721,7 +728,7 @@ func compareOneDriveItem(
err = json.Unmarshal(buf, &itemMeta) err = json.Unmarshal(buf, &itemMeta)
if !assert.NoErrorf(t, err, "unmarshalling retrieved metadata for file %s", name) { if !assert.NoErrorf(t, err, "unmarshalling retrieved metadata for file %s", name) {
return return true
} }
expectedData := expected[name] expectedData := expected[name]
@ -731,12 +738,12 @@ func compareOneDriveItem(
"unexpected metadata file with name %s", "unexpected metadata file with name %s",
name, name,
) { ) {
return return true
} }
err = json.Unmarshal(expectedData, &expectedMeta) err = json.Unmarshal(expectedData, &expectedMeta)
if !assert.NoError(t, err, "unmarshalling expected metadata") { if !assert.NoError(t, err, "unmarshalling expected metadata") {
return return true
} }
// Only compare file names if we're using a version that expects them to be // Only compare file names if we're using a version that expects them to be
@ -747,7 +754,7 @@ func compareOneDriveItem(
if !restorePermissions { if !restorePermissions {
assert.Equal(t, 0, len(itemMeta.Permissions)) assert.Equal(t, 0, len(itemMeta.Permissions))
return return true
} }
testElementsMatch( testElementsMatch(
@ -757,19 +764,19 @@ func compareOneDriveItem(
permissionEqual, permissionEqual,
) )
return return true
} }
var fileData testOneDriveData var fileData testOneDriveData
err = json.Unmarshal(buf, &fileData) err = json.Unmarshal(buf, &fileData)
if !assert.NoErrorf(t, err, "unmarshalling file data for file %s", name) { if !assert.NoErrorf(t, err, "unmarshalling file data for file %s", name) {
return return true
} }
expectedData := expected[fileData.FileName] expectedData := expected[fileData.FileName]
if !assert.NotNil(t, expectedData, "unexpected file with name %s", name) { if !assert.NotNil(t, expectedData, "unexpected file with name %s", name) {
return return true
} }
// OneDrive data items are just byte buffers of the data. Nothing special to // OneDrive data items are just byte buffers of the data. Nothing special to
@ -778,16 +785,22 @@ func compareOneDriveItem(
// Compare against the version with the file name embedded because that's what // Compare against the version with the file name embedded because that's what
// the auto-generated expected data has. // the auto-generated expected data has.
assert.Equal(t, expectedData, buf) assert.Equal(t, expectedData, buf)
return true
} }
// compareItem compares the data returned by backup with the expected data.
// Returns true if a comparison was done else false. Bool return is mostly used
// to exclude OneDrive permissions for the root right now.
func compareItem( func compareItem(
t *testing.T, t *testing.T,
expected map[string][]byte, expected map[string][]byte,
service path.ServiceType, service path.ServiceType,
category path.CategoryType, category path.CategoryType,
item data.Stream, item data.Stream,
dest control.RestoreDestination,
restorePermissions bool, restorePermissions bool,
) { ) bool {
if mt, ok := item.(data.StreamModTime); ok { if mt, ok := item.(data.StreamModTime); ok {
assert.NotZero(t, mt.ModTime()) assert.NotZero(t, mt.ModTime())
} }
@ -806,11 +819,13 @@ func compareItem(
} }
case path.OneDriveService: case path.OneDriveService:
compareOneDriveItem(t, expected, item, restorePermissions) return compareOneDriveItem(t, expected, item, dest, restorePermissions)
default: default:
assert.FailNowf(t, "unexpected service: %s", service.String()) assert.FailNowf(t, "unexpected service: %s", service.String())
} }
return true
} }
func checkHasCollections( func checkHasCollections(
@ -831,7 +846,7 @@ func checkHasCollections(
gotNames = append(gotNames, g.FullPath().String()) gotNames = append(gotNames, g.FullPath().String())
} }
assert.ElementsMatch(t, expectedNames, gotNames) assert.ElementsMatch(t, expectedNames, gotNames, "returned collections")
} }
//revive:disable:context-as-argument //revive:disable:context-as-argument
@ -841,6 +856,7 @@ func checkCollections(
expectedItems int, expectedItems int,
expected map[string]map[string][]byte, expected map[string]map[string][]byte,
got []data.BackupCollection, got []data.BackupCollection,
dest control.RestoreDestination,
restorePermissions bool, restorePermissions bool,
) int { ) int {
//revive:enable:context-as-argument //revive:enable:context-as-argument
@ -850,10 +866,12 @@ func checkCollections(
gotItems := 0 gotItems := 0
for _, returned := range got { for _, returned := range got {
startingItems := gotItems var (
service := returned.FullPath().Service() hasItems bool
category := returned.FullPath().Category() service = returned.FullPath().Service()
expectedColData := expected[returned.FullPath().String()] category = returned.FullPath().Category()
expectedColData = expected[returned.FullPath().String()]
)
// Need to iterate through all items even if we don't expect to find a match // Need to iterate through all items even if we don't expect to find a match
// because otherwise we'll deadlock waiting for GC status. Unexpected or // because otherwise we'll deadlock waiting for GC status. Unexpected or
@ -871,16 +889,19 @@ func checkCollections(
continue continue
} }
hasItems = true
gotItems++ gotItems++
if expectedColData == nil { if expectedColData == nil {
continue continue
} }
compareItem(t, expectedColData, service, category, item, restorePermissions) if !compareItem(t, expectedColData, service, category, item, dest, restorePermissions) {
gotItems--
}
} }
if gotItems != startingItems { if hasItems {
collectionsWithItems = append(collectionsWithItems, returned) collectionsWithItems = append(collectionsWithItems, returned)
} }
} }

View File

@ -490,7 +490,14 @@ func runBackupAndCompare(
// Pull the data prior to waiting for the status as otherwise it will // Pull the data prior to waiting for the status as otherwise it will
// deadlock. // deadlock.
skipped := checkCollections(t, ctx, totalKopiaItems, expectedData, dcs, config.opts.RestorePermissions) skipped := checkCollections(
t,
ctx,
totalKopiaItems,
expectedData,
dcs,
config.dest,
config.opts.RestorePermissions)
status := backupGC.AwaitStatus() status := backupGC.AwaitStatus()
@ -995,7 +1002,15 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames
// Pull the data prior to waiting for the status as otherwise it will // Pull the data prior to waiting for the status as otherwise it will
// deadlock. // deadlock.
skipped := checkCollections(t, ctx, allItems, allExpectedData, dcs, true) skipped := checkCollections(
t,
ctx,
allItems,
allExpectedData,
dcs,
// Alright to be empty, needed for OneDrive.
control.RestoreDestination{},
true)
status := backupGC.AwaitStatus() status := backupGC.AwaitStatus()
assert.Equal(t, allItems+skipped, status.ObjectCount, "status.ObjectCount") assert.Equal(t, allItems+skipped, status.ObjectCount, "status.ObjectCount")