Make ProcessTests.HandleCountChanges more reliable (dotnet#37330)

* Make ProcessTests.HandleCountChanges more reliable

* Address PR feedback

* Update CoreFx.Private.TestUtilities.csproj

src/CoreFx.Private.TestUtilities/ref/CoreFx.Private.TestUtilities.cs

@@ -136,6 +136,11 @@ public static partial class PlatformDetection
         public static int WindowsVersion { get { throw null; } }
         public static string GetDistroVersionString() { throw null; }
     }
+    public static partial class RetryHelper
+    {
+        public static void Execute(System.Action test, int maxAttempts = 5, System.Func<int, int> backoffFunc = null) { }
+        public static System.Threading.Tasks.Task ExecuteAsync(Func<System.Threading.Tasks.Task> test, int maxAttempts = 5, System.Func<int, int> backoffFunc = null) { throw null; }
+    }
     public static partial class TestEnvironment
     {
         public static bool IsStressModeEnabled { get { throw null; } }

src/CoreFx.Private.TestUtilities/src/CoreFx.Private.TestUtilities.csproj

@@ -34,6 +34,7 @@
       <Link>Common\CoreLib\System\PasteArguments.cs</Link>
     </Compile>
     <Compile Include="System\AssertExtensions.cs" />
+    <Compile Include="System\RetryHelper.cs" />
     <Compile Include="System\TheoryExtensions.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsWindows)' == 'true'">

src/CoreFx.Private.TestUtilities/src/System/RetryHelper.cs

@@ -0,0 +1,91 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace System
+{
+    public static partial class RetryHelper
+    {
+        private static readonly Func<int, int> s_defaultBackoffFunc = i => Math.Min(i * 100, 60_000);
+
+        /// <summary>Executes the <paramref name="test"/> action up to a maximum of <paramref name="maxAttempts"/> times.</summary>
+        /// <param name="maxAttempts">The maximum number of times to invoke <paramref name="test"/>.</param>
+        /// <param name="test">The test to invoke.</param>
+        /// <param name="backoffFunc">After a failure, invoked to determine how many milliseconds to wait before the next attempt.  It's passed the number of iterations attempted.</param>
+        public static void Execute(Action test, int maxAttempts = 5, Func<int, int> backoffFunc = null)
+        {
+            // Validate arguments
+            if (maxAttempts < 1)
+            {
+                throw new ArgumentOutOfRangeException(nameof(maxAttempts));
+            }
+            if (test == null)
+            {
+                throw new ArgumentNullException(nameof(test));
+            }
+
+            // Execute the test until it either passes or we run it maxAttempts times
+            var exceptions = new List<Exception>();
+            for (int i = 1; i <= maxAttempts; i++)
+            {
+                try
+                {
+                    test();
+                    return;
+                }
+                catch (Exception e)
+                {
+                    exceptions.Add(e);
+                    if (i == maxAttempts)
+                    {
+                        throw new AggregateException(exceptions);
+                    }
+                }
+
+                Thread.Sleep((backoffFunc ?? s_defaultBackoffFunc)(i));
+            }
+        }
+
+        /// <summary>Executes the <paramref name="test"/> action up to a maximum of <paramref name="maxAttempts"/> times.</summary>
+        /// <param name="maxAttempts">The maximum number of times to invoke <paramref name="test"/>.</param>
+        /// <param name="test">The test to invoke.</param>
+        /// <param name="backoffFunc">After a failure, invoked to determine how many milliseconds to wait before the next attempt.  It's passed the number of iterations attempted.</param>
+        public static async Task ExecuteAsync(Func<Task> test, int maxAttempts = 5, Func<int, int> backoffFunc = null)
+        {
+            // Validate arguments
+            if (maxAttempts < 1)
+            {
+                throw new ArgumentOutOfRangeException(nameof(maxAttempts));
+            }
+            if (test == null)
+            {
+                throw new ArgumentNullException(nameof(test));
+            }
+
+            // Execute the test until it either passes or we run it maxAttempts times
+            var exceptions = new List<Exception>();
+            for (int i = 1; i <= maxAttempts; i++)
+            {
+                try
+                {
+                    await test().ConfigureAwait(false);
+                    return;
+                }
+                catch (Exception e)
+                {
+                    exceptions.Add(e);
+                    if (i == maxAttempts)
+                    {
+                        throw new AggregateException(exceptions);
+                    }
+                }
+
+                await Task.Delay((backoffFunc ?? s_defaultBackoffFunc)(i)).ConfigureAwait(false);
+            }
+        }
+    }
+}

src/System.Diagnostics.Process/tests/ProcessModuleTests.cs

@@ -14,7 +14,7 @@ public class ProcessModuleTests : ProcessTestBase
         public void TestModuleProperties()
         {
             ProcessModuleCollection modules = Process.GetCurrentProcess().Modules;
-            Assert.True(modules.Count > 0);
+            Assert.InRange(modules.Count, 1, int.MaxValue);
 
             foreach (ProcessModule module in modules)
             {

src/System.Diagnostics.Process/tests/ProcessTests.cs

@@ -349,12 +349,7 @@ public void TestExitTime()
                     break;
             }
 
-            Assert.True(p.ExitTime.ToUniversalTime() >= timeBeforeProcessStart,
-                $@"TestExitTime is incorrect. " +
-                $@"TimeBeforeStart {timeBeforeProcessStart} Ticks={timeBeforeProcessStart.Ticks}, " +
-                $@"ExitTime={p.ExitTime}, Ticks={p.ExitTime.Ticks}, " +
-                $@"ExitTimeUniversal {p.ExitTime.ToUniversalTime()} Ticks={p.ExitTime.ToUniversalTime().Ticks}, " +
-                $@"NowUniversal {DateTime.Now.ToUniversalTime()} Ticks={DateTime.Now.Ticks}");
+            Assert.InRange(p.ExitTime.ToUniversalTime(), timeBeforeProcessStart, DateTime.MaxValue);
         }
 
         [Fact]
@@ -450,7 +445,7 @@ public void TestMainModule()
                 (Func<string, string>)((s) => s.ToLowerInvariant()) :
                 (s) => s;
 
-            Assert.True(p.Modules.Count > 0);
+            Assert.InRange(p.Modules.Count, 1, int.MaxValue);
             Assert.Equal(normalize(RemoteExecutor.HostRunnerName), normalize(p.MainModule.ModuleName));
             Assert.EndsWith(normalize(RemoteExecutor.HostRunnerName), normalize(p.MainModule.FileName));
             Assert.Equal(normalize(string.Format("System.Diagnostics.ProcessModule ({0})", RemoteExecutor.HostRunnerName)), normalize(p.MainModule.ToString()));
@@ -464,16 +459,16 @@ public void TestMaxWorkingSet()
 
             using (Process p = Process.GetCurrentProcess())
             {
-                Assert.True((long)p.MaxWorkingSet > 0);
-                Assert.True((long)p.MinWorkingSet >= 0);
+                Assert.InRange((long)p.MaxWorkingSet, 1, long.MaxValue);
+                Assert.InRange((long)p.MinWorkingSet, 0, long.MaxValue);
             }
 
             if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) || RuntimeInformation.IsOSPlatform(OSPlatform.Create("FREEBSD"))) {
                 return; // doesn't support getting/setting working set for other processes
             }
 
             long curValue = (long)_process.MaxWorkingSet;
-            Assert.True(curValue >= 0);
+            Assert.InRange(curValue, 0, long.MaxValue);
 
             if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
             {
@@ -520,16 +515,16 @@ public void TestMinWorkingSet()
 
             using (Process p = Process.GetCurrentProcess())
             {
-                Assert.True((long)p.MaxWorkingSet > 0);
-                Assert.True((long)p.MinWorkingSet >= 0);
+                Assert.InRange((long)p.MaxWorkingSet, 1, long.MaxValue);
+                Assert.InRange((long)p.MinWorkingSet, 0, long.MaxValue);
             }
 
             if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) || RuntimeInformation.IsOSPlatform(OSPlatform.Create("FREEBSD"))) {
                 return; // doesn't support getting/setting working set for other processes
             }
 
             long curValue = (long)_process.MinWorkingSet;
-            Assert.True(curValue >= 0);
+            Assert.InRange(curValue, 0, long.MaxValue);
 
             if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
             {
@@ -703,7 +698,7 @@ public void TestVirtualMemorySize64()
         {
             CreateDefaultProcess();
 
-            Assert.True(_process.VirtualMemorySize64 > 0);
+            Assert.InRange(_process.VirtualMemorySize64, 1, long.MaxValue);
         }
 
         [Fact]
@@ -722,11 +717,11 @@ public void TestWorkingSet64()
             if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
             {
                 // resident memory can be 0 on OSX.
-                Assert.True(_process.WorkingSet64 >= 0);
+                Assert.InRange(_process.WorkingSet64, 0, long.MaxValue);
                 return;
             }
 
-            Assert.True(_process.WorkingSet64 > 0);
+            Assert.InRange(_process.WorkingSet64, 1, long.MaxValue);
         }
 
         [Fact]
@@ -742,8 +737,8 @@ public void TestProcessorTime()
         {
             CreateDefaultProcess();
 
-            Assert.True(_process.UserProcessorTime.TotalSeconds >= 0);
-            Assert.True(_process.PrivilegedProcessorTime.TotalSeconds >= 0);
+            Assert.InRange(_process.UserProcessorTime.TotalSeconds, 0, long.MaxValue);
+            Assert.InRange(_process.PrivilegedProcessorTime.TotalSeconds, 0, long.MaxValue);
 
             double processorTimeBeforeSpin = Process.GetCurrentProcess().TotalProcessorTime.TotalSeconds;
             double processorTimeAtHalfSpin = 0;
@@ -1457,7 +1452,7 @@ public void TestHandleCount()
         {
             using (Process p = Process.GetCurrentProcess())
             {
-                Assert.True(p.HandleCount > 0);
+                Assert.InRange(p.HandleCount, 1, int.MaxValue);
             }
         }
 
@@ -1471,7 +1466,7 @@ public void TestHandleCount_OSX()
             }
         }
 
-        [ActiveIssue(37325)]
+        [OuterLoop]
         [Fact]
         [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.Windows)]  // Expected process HandleCounts differs on OSX
         [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Handle count change is not reliable, but seems less robust on NETFX")]
@@ -1480,20 +1475,41 @@ public void HandleCountChanges()
         {
             RemoteExecutor.Invoke(() =>
             {
-                Process p = Process.GetCurrentProcess();
-                int handleCount = p.HandleCount;
-                using (var fs1 = File.Open(GetTestFilePath(), FileMode.OpenOrCreate))
-                using (var fs2 = File.Open(GetTestFilePath(), FileMode.OpenOrCreate))
-                using (var fs3 = File.Open(GetTestFilePath(), FileMode.OpenOrCreate))
+                RetryHelper.Execute(() =>
                 {
-                    p.Refresh();
-                    int secondHandleCount = p.HandleCount;
-                    Assert.True(handleCount < secondHandleCount);
-                    handleCount = secondHandleCount;
-                }
-                p.Refresh();
-                int thirdHandleCount = p.HandleCount;
-                Assert.True(thirdHandleCount < handleCount);
+                    using (Process p = Process.GetCurrentProcess())
+                    {
+                        // Warm up code paths
+                        p.Refresh();
+                        using (var tmpFile = File.Open(GetTestFilePath(), FileMode.OpenOrCreate))
+                        {
+                            // Get the initial handle count
+                            p.Refresh();
+                            int handleCountAtStart = p.HandleCount;
+                            int handleCountAfterOpens;
+
+                            // Open a bunch of files and get a new handle count, then close the files
+                            var files = new List<FileStream>();
+                            try
+                            {
+                                files.AddRange(Enumerable.Range(0, 50).Select(_ => File.Open(GetTestFilePath(), FileMode.OpenOrCreate)));
+                                p.Refresh();
+                                handleCountAfterOpens = p.HandleCount;
+                            }
+                            finally
+                            {
+                                files.ForEach(f => f.Dispose());
+                            }
+
+                            // Get the handle count after closing all the files
+                            p.Refresh();
+                            int handleCountAtEnd = p.HandleCount;
+
+                            Assert.InRange(handleCountAfterOpens, handleCountAtStart + 1, int.MaxValue);
+                            Assert.InRange(handleCountAtEnd, handleCountAtStart, handleCountAfterOpens - 1);
+                        }
+                    }
+                });
                 return RemoteExecutor.SuccessExitCode;
             }).Dispose();
         }
@@ -1762,13 +1778,13 @@ public void TestWorkingSet()
             {
                 // resident memory can be 0 on OSX.
 #pragma warning disable 0618
-                Assert.True(_process.WorkingSet >= 0);
+                Assert.InRange(_process.WorkingSet, 0, int.MaxValue);
 #pragma warning restore 0618
                 return;
             }
 
 #pragma warning disable 0618
-            Assert.True(_process.WorkingSet > 0);
+            Assert.InRange(_process.WorkingSet, 1, int.MaxValue);
 #pragma warning restore 0618
         }
 

src/System.Diagnostics.Process/tests/ProcessThreadTests.cs

@@ -20,7 +20,7 @@ public void TestCommonPriorityAndTimeProperties()
             CreateDefaultProcess();
 
             ProcessThreadCollection threadCollection = _process.Threads;
-            Assert.True(threadCollection.Count > 0);
+            Assert.InRange(threadCollection.Count, 1, int.MaxValue);
             ProcessThread thread = threadCollection[0];
             try
             {
@@ -29,12 +29,15 @@ public void TestCommonPriorityAndTimeProperties()
                     // On OSX, thread id is a 64bit unsigned value. We truncate the ulong to int
                     // due to .NET API surface area. Hence, on overflow id can be negative while
                     // casting the ulong to int.
-                    Assert.True(thread.Id >= 0 || RuntimeInformation.IsOSPlatform(OSPlatform.OSX));
+                    if (!RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
+                    {
+                        Assert.InRange(thread.Id, 0, int.MaxValue);
+                    }
                     Assert.Equal(_process.BasePriority, thread.BasePriority);
-                    Assert.True(thread.CurrentPriority >= 0);
-                    Assert.True(thread.PrivilegedProcessorTime.TotalSeconds >= 0);
-                    Assert.True(thread.UserProcessorTime.TotalSeconds >= 0);
-                    Assert.True(thread.TotalProcessorTime.TotalSeconds >= 0);
+                    Assert.InRange(thread.CurrentPriority, 0, int.MaxValue);
+                    Assert.InRange(thread.PrivilegedProcessorTime.TotalSeconds, 0, int.MaxValue);
+                    Assert.InRange(thread.UserProcessorTime.TotalSeconds, 0, int.MaxValue);
+                    Assert.InRange(thread.TotalProcessorTime.TotalSeconds, 0, int.MaxValue);
                 }
             }
             catch (Exception e) when (e is Win32Exception || e is InvalidOperationException)
@@ -120,7 +123,7 @@ public async Task TestStartTimeProperty()
                         // The thread may have gone away between our getting its info and attempting to access its StartTime
                     }
                 }
-                Assert.True(passed > 0, "Expected at least one thread to be valid for StartTime");
+                Assert.InRange(passed, 1, int.MaxValue);
 
                 // Now add a thread, and from that thread, while it's still alive, verify
                 // that there's at least one thread greater than the current time we previously grabbed.