From af977f46ca524540274c186e985fec4b74fbfb39 Mon Sep 17 00:00:00 2001
From: ashmrtn <ashmrtnz@alcion.ai>
Date: Thu, 1 Dec 2022 19:39:45 -0800
Subject: [PATCH] Fix OneDrive and Kopia Wrapper test flakes (#1662)

## Description

Minor fixes for flakey tests:

* Dedupe OneDrive folders by checking IDs. Should help remove some test flakes.
* Weaken checks on kopia snapshot manager tests as they use maps with indeterminate iteration orders

## Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] :sunflower: Feature
- [x] :bug: Bugfix
- [ ] :world_map: Documentation
- [ ] :robot: Test
- [ ] :computer: CI/Deployment
- [ ] :hamster: Trivial/Minor

## Issue(s)

* closes #1660
* closes #1657

## Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] :muscle: Manual
- [x] :zap: Unit test
- [ ] :green_heart: E2E
---
 src/internal/connector/onedrive/drive.go    | 15 +++++++++++++--
 src/internal/kopia/snapshot_manager_test.go |  6 +++---
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/internal/connector/onedrive/drive.go b/src/internal/connector/onedrive/drive.go
index 3517fc2d8..7a8fce3f3 100644
--- a/src/internal/connector/onedrive/drive.go
+++ b/src/internal/connector/onedrive/drive.go
@@ -275,7 +275,7 @@ func GetAllFolders(
 		return nil, errors.Wrap(err, "getting OneDrive folders")
 	}
 
-	res := []*Displayable{}
+	folders := map[string]*Displayable{}
 
 	for _, d := range drives {
 		err = collectItems(
@@ -294,13 +294,18 @@ func GetAllFolders(
 						continue
 					}
 
+					if item.GetId() == nil || len(*item.GetId()) == 0 {
+						logger.Ctx(ctx).Warn("folder without ID")
+						continue
+					}
+
 					if !strings.HasPrefix(*item.GetName(), prefix) {
 						continue
 					}
 
 					// Add the item instead of the folder because the item has more
 					// functionality.
-					res = append(res, &Displayable{item})
+					folders[*item.GetId()] = &Displayable{item}
 				}
 
 				return nil
@@ -311,6 +316,12 @@ func GetAllFolders(
 		}
 	}
 
+	res := make([]*Displayable, 0, len(folders))
+
+	for _, f := range folders {
+		res = append(res, f)
+	}
+
 	return res, nil
 }
 
diff --git a/src/internal/kopia/snapshot_manager_test.go b/src/internal/kopia/snapshot_manager_test.go
index cfd046f01..015c000e6 100644
--- a/src/internal/kopia/snapshot_manager_test.go
+++ b/src/internal/kopia/snapshot_manager_test.go
@@ -526,8 +526,6 @@ func (suite *SnapshotFetchUnitSuite) TestFetchPrevSnapshotsWorksWithErrors() {
 		),
 	}
 
-	expected := []*snapshot.Manifest{mockData[2].man}
-
 	msm := &mockErrorSnapshotManager{
 		sm: &mockSnapshotManager{
 			data: mockData,
@@ -536,5 +534,7 @@ func (suite *SnapshotFetchUnitSuite) TestFetchPrevSnapshotsWorksWithErrors() {
 
 	snaps := fetchPrevSnapshotManifests(ctx, msm, input)
 
-	assert.ElementsMatch(t, expected, snaps)
+	// Only 1 snapshot should be chosen because the other two attempts fail.
+	// However, which one is returned is non-deterministic because maps are used.
+	assert.Len(t, snaps, 1)
 }