From 243f92aa478c4fc82ba5450dac48e8750ae3019d Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 19 Jan 2017 11:37:34 -0800 Subject: [PATCH 1/3] fix some flakiness around test assets --- .../TestAssetInfo.cs | 46 +++++-------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs b/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs index a11e149cb..47047fd6f 100644 --- a/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs +++ b/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs @@ -140,18 +140,16 @@ namespace Microsoft.DotNet.TestFramework private IEnumerable LoadInventory(FileInfo file) { + file.Refresh(); if (!file.Exists) { - return Enumerable.Empty(); + throw new InvalidOperationException("Inventory file should exist."); } var inventory = new List(); - - var lines = file.OpenText(); - - while (lines.Peek() > 0) + foreach (var p in File.ReadAllLines(file.FullName)) { - inventory.Add(new FileInfo(lines.ReadLine())); + inventory.Add(new FileInfo(p)); } return inventory; @@ -159,27 +157,13 @@ namespace Microsoft.DotNet.TestFramework private void SaveInventory(FileInfo file, IEnumerable inventory) { - FileUtility.ReplaceWithLock( - filePath => - { - if (!_dataDirectory.Exists) - { - _dataDirectory.Create(); - } + _dataDirectory.Refresh(); + if (!_dataDirectory.Exists) + { + _dataDirectory.Create(); + } - using (var stream = - new FileStream(filePath, FileMode.Create, FileAccess.Write, FileShare.None)) - { - using (var writer = new StreamWriter(stream)) - { - foreach (var path in inventory.Select(i => i.FullName)) - { - writer.WriteLine(path); - } - } - } - }, - file.FullName); + File.WriteAllLines(file.FullName, inventory.Select((fi) => fi.FullName).ToList()); } private IEnumerable GetFileList() @@ -195,15 +179,6 @@ namespace Microsoft.DotNet.TestFramework Action action) { var inventory = Enumerable.Empty(); - if (file.Exists) - { - inventory = LoadInventory(file); - } - - if(inventory.Any()) - { - return inventory; - } IEnumerable preInventory; @@ -220,6 +195,7 @@ namespace Microsoft.DotNet.TestFramework _dataDirectory.FullName, lockedToken => { + file.Refresh(); if (file.Exists) { inventory = LoadInventory(file); From e271b4a8184520bf0bec78b0239e074bfe6a055b Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 19 Jan 2017 15:56:16 -0800 Subject: [PATCH 2/3] InvalidOperationException -> ArgumentException --- src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs b/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs index 47047fd6f..8d82e3b3f 100644 --- a/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs +++ b/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs @@ -143,7 +143,7 @@ namespace Microsoft.DotNet.TestFramework file.Refresh(); if (!file.Exists) { - throw new InvalidOperationException("Inventory file should exist."); + throw new ArgumentException("Inventory file should exist."); } var inventory = new List(); From 8ef670161b157ab2242c58b5a7b1ca95d2d5fa6d Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 19 Jan 2017 16:43:54 -0800 Subject: [PATCH 3/3] isolate exclusive folder access pattern into separate class --- .../TestAssetInfo.cs | 125 +++++++++++------- 1 file changed, 77 insertions(+), 48 deletions(-) diff --git a/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs b/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs index 8d82e3b3f..e1bcbcbf8 100644 --- a/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs +++ b/src/Microsoft.DotNet.TestFramework/TestAssetInfo.cs @@ -138,34 +138,6 @@ namespace Microsoft.DotNet.TestFramework return new DirectoryInfo(Path.Combine(baseDirectory, callingMethod + identifier, _assetName)); } - private IEnumerable LoadInventory(FileInfo file) - { - file.Refresh(); - if (!file.Exists) - { - throw new ArgumentException("Inventory file should exist."); - } - - var inventory = new List(); - foreach (var p in File.ReadAllLines(file.FullName)) - { - inventory.Add(new FileInfo(p)); - } - - return inventory; - } - - private void SaveInventory(FileInfo file, IEnumerable inventory) - { - _dataDirectory.Refresh(); - if (!_dataDirectory.Exists) - { - _dataDirectory.Create(); - } - - File.WriteAllLines(file.FullName, inventory.Select((fi) => fi.FullName).ToList()); - } - private IEnumerable GetFileList() { return _root.GetFiles("*.*", SearchOption.AllDirectories) @@ -191,27 +163,21 @@ namespace Microsoft.DotNet.TestFramework preInventory = beforeAction(); } - Task.Run(async () => await ConcurrencyUtilities.ExecuteWithFileLockedAsync( - _dataDirectory.FullName, - lockedToken => + ExclusiveFolderAccess.Do(_dataDirectory, (folder) => { + file.Refresh(); + if (file.Exists) { - file.Refresh(); - if (file.Exists) - { - inventory = LoadInventory(file); - } - else - { - action(); + inventory = folder.LoadInventory(file); + } + else + { + action(); - inventory = GetFileList().Where(i => !preInventory.Select(p => p.FullName).Contains(i.FullName)); + inventory = GetFileList().Where(i => !preInventory.Select(p => p.FullName).Contains(i.FullName)); - SaveInventory(file, inventory); - } - - return Task.FromResult(new Object()); - }, - CancellationToken.None)).Wait(); + folder.SaveInventory(file, inventory); + } + }); return inventory; } @@ -286,7 +252,10 @@ namespace Microsoft.DotNet.TestFramework return; } - var trackedFiles = _inventoryFiles.AllInventoryFiles.SelectMany(f => LoadInventory(f)); + IEnumerable trackedFiles = null; + ExclusiveFolderAccess.Do(_dataDirectory, (folder) => { + trackedFiles = _inventoryFiles.AllInventoryFiles.SelectMany(f => folder.LoadInventory(f)); + }); var assetFiles = GetFileList(); @@ -316,7 +285,7 @@ namespace Microsoft.DotNet.TestFramework return; } - var updatedSourceFiles = LoadInventory(_inventoryFiles.Source) + var updatedSourceFiles = ExclusiveFolderAccess.Read(_inventoryFiles.Source) .Where(f => f.LastWriteTime > earliestDataDirectoryTimestamp); if (updatedSourceFiles.Any()) @@ -340,5 +309,65 @@ namespace Microsoft.DotNet.TestFramework throw new DirectoryNotFoundException($"Directory not found at '{_root.FullName}'"); } } + + private class ExclusiveFolderAccess + { + private DirectoryInfo _directory; + + private ExclusiveFolderAccess(DirectoryInfo directory) + { + _directory = directory; + } + + public static void Do(DirectoryInfo directory, Action action) + { + Task.Run(async () => await ConcurrencyUtilities.ExecuteWithFileLockedAsync( + directory.FullName, + lockedToken => + { + action(new ExclusiveFolderAccess(directory)); + return Task.FromResult(new Object()); + }, + CancellationToken.None)).Wait(); + } + + public static IEnumerable Read(FileInfo file) + { + IEnumerable ret = null; + Do(file.Directory, (folder) => { + ret = folder.LoadInventory(file); + }); + + return ret; + } + + public IEnumerable LoadInventory(FileInfo file) + { + file.Refresh(); + if (!file.Exists) + { + throw new ArgumentException("Inventory file should exist."); + } + + var inventory = new List(); + foreach (var p in File.ReadAllLines(file.FullName)) + { + inventory.Add(new FileInfo(p)); + } + + return inventory; + } + + public void SaveInventory(FileInfo file, IEnumerable inventory) + { + _directory.Refresh(); + if (!_directory.Exists) + { + _directory.Create(); + } + + File.WriteAllLines(file.FullName, inventory.Select((fi) => fi.FullName).ToList()); + } + } } }