From d398edffd620fd3e4136963faedfea3a9a8e66b0 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 1 May 2019 21:49:37 -0400 Subject: [PATCH] Make ProcessTests.HandleCountChanges more reliable (#37330) * Make ProcessTests.HandleCountChanges more reliable * Address PR feedback * Update CoreFx.Private.TestUtilities.csproj --- .../ref/CoreFx.Private.TestUtilities.cs | 5 + .../src/CoreFx.Private.TestUtilities.csproj | 1 + .../src/System/RetryHelper.cs | 91 +++++++++++++++++++ .../tests/ProcessModuleTests.cs | 2 +- .../tests/ProcessTests.cs | 86 +++++++++++------- .../tests/ProcessThreadTests.cs | 17 ++-- 6 files changed, 159 insertions(+), 43 deletions(-) create mode 100644 src/CoreFx.Private.TestUtilities/src/System/RetryHelper.cs diff --git a/src/CoreFx.Private.TestUtilities/ref/CoreFx.Private.TestUtilities.cs b/src/CoreFx.Private.TestUtilities/ref/CoreFx.Private.TestUtilities.cs index 8f457aab11c8..ff5dc9cf1e06 100644 --- a/src/CoreFx.Private.TestUtilities/ref/CoreFx.Private.TestUtilities.cs +++ b/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 backoffFunc = null) { } + public static System.Threading.Tasks.Task ExecuteAsync(Func test, int maxAttempts = 5, System.Func backoffFunc = null) { throw null; } + } public static partial class TestEnvironment { public static bool IsStressModeEnabled { get { throw null; } } diff --git a/src/CoreFx.Private.TestUtilities/src/CoreFx.Private.TestUtilities.csproj b/src/CoreFx.Private.TestUtilities/src/CoreFx.Private.TestUtilities.csproj index 1112b71aae4a..cda53cdc7c11 100644 --- a/src/CoreFx.Private.TestUtilities/src/CoreFx.Private.TestUtilities.csproj +++ b/src/CoreFx.Private.TestUtilities/src/CoreFx.Private.TestUtilities.csproj @@ -34,6 +34,7 @@ Common\CoreLib\System\PasteArguments.cs + diff --git a/src/CoreFx.Private.TestUtilities/src/System/RetryHelper.cs b/src/CoreFx.Private.TestUtilities/src/System/RetryHelper.cs new file mode 100644 index 000000000000..0308f11cea5d --- /dev/null +++ b/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 s_defaultBackoffFunc = i => Math.Min(i * 100, 60_000); + + /// Executes the action up to a maximum of times. + /// The maximum number of times to invoke . + /// The test to invoke. + /// After a failure, invoked to determine how many milliseconds to wait before the next attempt. It's passed the number of iterations attempted. + public static void Execute(Action test, int maxAttempts = 5, Func 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(); + 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)); + } + } + + /// Executes the action up to a maximum of times. + /// The maximum number of times to invoke . + /// The test to invoke. + /// After a failure, invoked to determine how many milliseconds to wait before the next attempt. It's passed the number of iterations attempted. + public static async Task ExecuteAsync(Func test, int maxAttempts = 5, Func 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(); + 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); + } + } + } +} diff --git a/src/System.Diagnostics.Process/tests/ProcessModuleTests.cs b/src/System.Diagnostics.Process/tests/ProcessModuleTests.cs index 1c6f06fc60cd..b11d5b0eaf17 100644 --- a/src/System.Diagnostics.Process/tests/ProcessModuleTests.cs +++ b/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) { diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.cs b/src/System.Diagnostics.Process/tests/ProcessTests.cs index 2f19b9141713..31f560475434 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/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)((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,8 +459,8 @@ 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"))) { @@ -473,7 +468,7 @@ public void TestMaxWorkingSet() } long curValue = (long)_process.MaxWorkingSet; - Assert.True(curValue >= 0); + Assert.InRange(curValue, 0, long.MaxValue); if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { @@ -520,8 +515,8 @@ 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"))) { @@ -529,7 +524,7 @@ public void TestMinWorkingSet() } 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(); + 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 } diff --git a/src/System.Diagnostics.Process/tests/ProcessThreadTests.cs b/src/System.Diagnostics.Process/tests/ProcessThreadTests.cs index df00d40b0317..0af036547b4e 100644 --- a/src/System.Diagnostics.Process/tests/ProcessThreadTests.cs +++ b/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.