Fix flaky ModifiedPathsTests by replaying the on-disk log

GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs

@@ -248,7 +248,7 @@ public void CheckoutBranchAfterReadingFileAndVerifyContentsCorrect()
             this.FilesShouldMatchCheckoutOfSourceBranch();
 
             // Verify modified paths contents
-            GVFSHelpers.ModifiedPathsContentsShouldEqual(this.Enlistment, this.FileSystem, "A .gitattributes" + GVFSHelpers.ModifiedPathsNewLine);
+            GVFSHelpers.ModifiedPathsShouldOnlyContain(this.Enlistment, this.FileSystem, ".gitattributes");
         }
 
         [TestCase]
@@ -266,7 +266,7 @@ public void CheckoutBranchAfterReadingAllFilesAndVerifyContentsCorrect()
                 .WithDeepStructure(this.FileSystem, this.ControlGitRepo.RootPath, compareContent: true, withinPrefixes: this.pathPrefixes);
 
             // Verify modified paths contents
-            GVFSHelpers.ModifiedPathsContentsShouldEqual(this.Enlistment, this.FileSystem, "A .gitattributes" + GVFSHelpers.ModifiedPathsNewLine);
+            GVFSHelpers.ModifiedPathsShouldOnlyContain(this.Enlistment, this.FileSystem, ".gitattributes");
         }
 
         [TestCase]

GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs

@@ -165,34 +165,35 @@ public static string ReadAllTextFromWriteLockedFile(string filename)
             }
         }
 
-        public static void ModifiedPathsContentsShouldEqual(GVFSFunctionalTestEnlistment enlistment, FileSystemRunner fileSystem, string contents)
+        /// <summary>
+        /// Asserts that the modified-paths set, after replaying the on-disk
+        /// A/D log, contains exactly <paramref name="gitPaths"/> -- no more,
+        /// no fewer. Use this when a test wants to prove that some sequence
+        /// of operations produced no spurious modified-paths entries.
+        /// </summary>
+        public static void ModifiedPathsShouldOnlyContain(GVFSFunctionalTestEnlistment enlistment, FileSystemRunner fileSystem, params string[] gitPaths)
         {
-            string modifedPathsContents = GetModifiedPathsContents(enlistment, fileSystem);
-            modifedPathsContents.ShouldEqual(contents);
+            HashSet<string> currentPaths = GetCurrentModifiedPaths(enlistment, fileSystem);
+            HashSet<string> expectedPaths = new HashSet<string>(gitPaths, FileSystemHelpers.PathComparer);
+            currentPaths.SetEquals(expectedPaths).ShouldBeTrue(
+                $"Expected modified paths {{{string.Join(",", expectedPaths)}}} but got {{{string.Join(",", currentPaths)}}}");
         }
 
         public static void ModifiedPathsShouldContain(GVFSFunctionalTestEnlistment enlistment, FileSystemRunner fileSystem, params string[] gitPaths)
         {
-            string modifedPathsContents = GetModifiedPathsContents(enlistment, fileSystem);
-            string[] modifedPathLines = modifedPathsContents.Split(new[] { ModifiedPathsNewLine }, StringSplitOptions.None);
+            HashSet<string> currentPaths = GetCurrentModifiedPaths(enlistment, fileSystem);
             foreach (string gitPath in gitPaths)
             {
-                modifedPathLines.ShouldContain(path => path.Equals(ModifedPathsLineAddPrefix + gitPath, FileSystemHelpers.PathComparison));
+                currentPaths.ShouldContain(path => path.Equals(gitPath, FileSystemHelpers.PathComparison));
             }
         }
 
         public static void ModifiedPathsShouldNotContain(GVFSFunctionalTestEnlistment enlistment, FileSystemRunner fileSystem, params string[] gitPaths)
         {
-            string modifedPathsContents = GetModifiedPathsContents(enlistment, fileSystem);
-            string[] modifedPathLines = modifedPathsContents.Split(new[] { ModifiedPathsNewLine }, StringSplitOptions.None);
+            HashSet<string> currentPaths = GetCurrentModifiedPaths(enlistment, fileSystem);
             foreach (string gitPath in gitPaths)
             {
-                modifedPathLines.ShouldNotContain(
-                    path =>
-                    {
-                        return path.Equals(ModifedPathsLineAddPrefix + gitPath, FileSystemHelpers.PathComparison) ||
-                               path.Equals(ModifedPathsLineDeletePrefix + gitPath, FileSystemHelpers.PathComparison);
-                    });
+                currentPaths.ShouldNotContain(path => path.Equals(gitPath, FileSystemHelpers.PathComparison));
             }
         }
 
@@ -230,6 +231,36 @@ private static string GetModifiedPathsContents(GVFSFunctionalTestEnlistment enli
             return GVFSHelpers.ReadAllTextFromWriteLockedFile(modifiedPathsDatabase);
         }
 
+        /// <summary>
+        /// Returns the set of currently-modified paths by replaying the on-disk
+        /// modified paths log. The file is append-only between background-op
+        /// batches; <see cref="ModifiedPathsDatabase.WriteAllEntriesAndFlush"/>
+        /// compacts it after each batch finishes. Because
+        /// <see cref="GVFSFunctionalTestEnlistment.WaitForBackgroundOperations"/>
+        /// can return after the last task is dequeued but before that compaction
+        /// completes, callers must replay the A/D log entries the same way
+        /// <see cref="FileBasedCollection.TryLoadFromDisk"/> does on mount to
+        /// observe a consistent state.
+        /// </summary>
+        private static HashSet<string> GetCurrentModifiedPaths(GVFSFunctionalTestEnlistment enlistment, FileSystemRunner fileSystem)
+        {
+            string contents = GetModifiedPathsContents(enlistment, fileSystem);
+            HashSet<string> paths = new HashSet<string>(FileSystemHelpers.PathComparer);
+            foreach (string line in contents.Split(new[] { ModifiedPathsNewLine }, StringSplitOptions.RemoveEmptyEntries))
+            {
+                if (line.StartsWith(ModifedPathsLineAddPrefix, StringComparison.Ordinal))
+                {
+                    paths.Add(line.Substring(ModifedPathsLineAddPrefix.Length));
+                }
+                else if (line.StartsWith(ModifedPathsLineDeletePrefix, StringComparison.Ordinal))
+                {
+                    paths.Remove(line.Substring(ModifedPathsLineDeletePrefix.Length));
+                }
+            }
+
+            return paths;
+        }
+
         private static T RunSqliteCommand<T>(string sqliteDbPath, Func<SqliteCommand, T> runCommand)
         {
             string connectionString = $"data source={sqliteDbPath}";

GVFS/GVFS.Tests/Should/EnumerableShouldExtensions.cs

@@ -47,8 +47,15 @@ public static T ShouldContainSingle<T>(this IEnumerable<T> group, Func<T, bool>
 
         public static void ShouldNotContain<T>(this IEnumerable<T> group, Func<T, bool> predicate)
         {
-            T item = group.SingleOrDefault(predicate);
-            item.ShouldEqual(default(T), "Unexpected matching entry found in {" + string.Join(",", group) + "}");
+            List<T> matches = group.Where(predicate).ToList();
+            if (matches.Count != 0)
+            {
+                Assert.Fail("Unexpected matching {0} {1}: {2} found in {{{3}}}",
+                    matches.Count,
+                    matches.Count == 1 ? "entry" : "entries",
+                    string.Join(",", matches),
+                    string.Join(",", group));
+            }
         }
 
         public static IEnumerable<T> ShouldNotContain<T>(this IEnumerable<T> group, IEnumerable<T> unexpectedValues, Func<T, T, bool> predicate)

GVFS/GVFS.UnitTests/Common/ModifiedPathsDatabaseTests.cs

@@ -135,6 +135,24 @@ public void EntryNotAddedIfParentDirectoryExists()
             modifiedPathsDatabase.Contains("dir2/dir", isFolder: true).ShouldBeTrue();
         }
 
+        [TestCase]
+        public void AddFollowedByDeleteIsRecoveredOnLoad()
+        {
+            // Simulates the on-disk state during the window between a background
+            // operation completing and PostBackgroundOperation calling
+            // WriteAllEntriesAndFlush. The append log contains both the add and
+            // delete entries; a subsequent load must replay them and end with
+            // the path NOT in the modified-paths set.
+            const string AddThenDelete = "A temp.txt\r\nD temp.txt\r\n";
+
+            ModifiedPathsDatabase modifiedPathsDatabase = CreateModifiedPathsDatabase(AddThenDelete);
+
+            // Only the auto-added .gitattributes default entry should remain.
+            modifiedPathsDatabase.Count.ShouldEqual(1);
+            modifiedPathsDatabase.Contains(DefaultEntry, isFolder: false).ShouldBeTrue();
+            modifiedPathsDatabase.Contains("temp.txt", isFolder: false).ShouldBeFalse();
+        }
+
         [TestCase]
         public void RemoveEntriesWithParentFolderEntry()
         {