From 0b1f521d81fc5a84983d253e5f18b44d73284383 Mon Sep 17 00:00:00 2001 From: Rainer Sigwald Date: Mon, 6 Apr 2026 11:29:39 -0500 Subject: [PATCH 1/4] Use ping.exe in ToolTask sleep helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Utilities.UnitTests/ToolTask_Tests.cs | 47 ++++++++++++----------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/Utilities.UnitTests/ToolTask_Tests.cs b/src/Utilities.UnitTests/ToolTask_Tests.cs index 463bacfc033..fc4cf4c0ba2 100644 --- a/src/Utilities.UnitTests/ToolTask_Tests.cs +++ b/src/Utilities.UnitTests/ToolTask_Tests.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics; +using System.Globalization; using System.IO; using System.Resources; using System.Text.RegularExpressions; @@ -1106,25 +1107,21 @@ public void ToolTaskCapturesAllOutputWithFix() /// A simple implementation of to sleep for a while. /// /// - /// This task runs shell command to sleep for predefined, variable amount of time based on how many times the - /// instance has been executed. + /// This task invokes a direct sleep tool with a variable delay based on how many times the instance has been + /// executed, which avoids the flakiness of nesting the wait inside a shell. /// private sealed class ToolTaskThatSleeps : ToolTask { - // Windows prompt command to sleep: - private readonly string _windowsSleep = "/c start /wait timeout {0}"; - - // UNIX command to sleep: - private readonly string _unixSleep = "-c \"sleep {0}\""; - - // Full path to shell: - private readonly string _pathToShell; + private readonly string _pathToTool; public ToolTaskThatSleeps() : base() { - // Determines shell to use: cmd for Windows, sh for UNIX-like systems: - _pathToShell = NativeMethodsShared.IsUnixLike ? "/bin/sh" : "cmd.exe"; + // timeout.exe exits immediately when ToolTask redirects stdin on Windows, so use ping.exe + // as the built-in blocking process for the timeout/retry scenario. + _pathToTool = NativeMethodsShared.IsUnixLike + ? "sleep" + : Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "ping.exe"); } /// @@ -1139,9 +1136,9 @@ public ToolTaskThatSleeps() /// Gets or sets the delay for the follow-up executions. /// /// - /// Defaults to 1 milliseconds. + /// Defaults to 100 milliseconds. /// - public Int32 FollowupDelay { get; set; } = 1; + public Int32 FollowupDelay { get; set; } = 100; /// /// Int32 output parameter for the repeat counter for test purpose. @@ -1150,22 +1147,26 @@ public ToolTaskThatSleeps() public Int32 RepeatCount { get; private set; } = 0; /// - /// Gets the tool name (shell). + /// Gets the tool name. /// - protected override string ToolName => Path.GetFileName(_pathToShell); + protected override string ToolName => Path.GetFileName(_pathToTool); /// - /// Gets the full path to shell. + /// Gets the full path to the sleep tool. /// - protected override string GenerateFullPathToTool() => _pathToShell; + protected override string GenerateFullPathToTool() => _pathToTool; /// - /// Generates a shell command to sleep different amount of time based on repeat counter. + /// Generates the arguments to sleep for a different amount of time based on repeat counter. /// - protected override string GenerateCommandLineCommands() => - NativeMethodsShared.IsUnixLike ? - string.Format(_unixSleep, RepeatCount < 2 ? InitialDelay / 1000.0 : FollowupDelay / 1000.0) : - string.Format(_windowsSleep, RepeatCount < 2 ? InitialDelay / 1000.0 : FollowupDelay / 1000.0); + protected override string GenerateCommandLineCommands() + { + int delay = RepeatCount < 2 ? InitialDelay : FollowupDelay; + + return NativeMethodsShared.IsUnixLike + ? (delay / 1000.0).ToString("0.###", CultureInfo.InvariantCulture) + : $"-n {Math.Max(2, (int)Math.Ceiling(delay / 1000.0) + 1).ToString(CultureInfo.InvariantCulture)} 127.0.0.1"; + } /// /// Ensures that test parameters make sense. From 47ad85815cb8b3858a7eeedc559ef181f9ff2f88 Mon Sep 17 00:00:00 2001 From: Rainer Sigwald Date: Mon, 6 Apr 2026 11:35:17 -0500 Subject: [PATCH 2/4] Refactor ToolTask timeout retry test Make the retry test assert the timeout scenario explicitly, log per-attempt diagnostics, and keep the follow-up executions deterministic across retries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Utilities.UnitTests/ToolTask_Tests.cs | 57 ++++++++++++++--------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/Utilities.UnitTests/ToolTask_Tests.cs b/src/Utilities.UnitTests/ToolTask_Tests.cs index fc4cf4c0ba2..eb35569d301 100644 --- a/src/Utilities.UnitTests/ToolTask_Tests.cs +++ b/src/Utilities.UnitTests/ToolTask_Tests.cs @@ -21,7 +21,7 @@ namespace Microsoft.Build.UnitTests { public sealed class ToolTask_Tests { - private ITestOutputHelper _output; + private readonly ITestOutputHelper _output; public ToolTask_Tests(ITestOutputHelper testOutput) { @@ -994,50 +994,61 @@ public void SetsTerminationTimeoutCorrectly(int timeout, bool isInvalidValid) /// Verifies that a ToolTask instance can return correct results when executed multiple times with timeout. /// /// Specifies the number of repeats for external command execution. - /// Delay to generate on the first execution in milliseconds. - /// Delay to generate on follow-up execution in milliseconds. - /// Task timeout in milliseconds. + /// Whether the first execution should be forced to time out before later retries succeed. /// - /// These tests execute the same task instance multiple times, which will in turn run a shell command to sleep - /// predefined amount of time. The first execution may time out, but all following ones won't. It is expected - /// that all following executions return success. + /// These tests execute the same task instance multiple times using a direct OS sleep process rather than + /// shelling through PowerShell or cmd.exe. When configured to time out, the first execution is expected to + /// fail and all following executions must still succeed. /// [Theory] - [InlineData(1, 1, 1, -1)] // Normal case, no repeat. - [InlineData(3, 1, 1, -1)] // Repeat without timeout. - [InlineData(3, 10000, 1, 1000)] // Repeat with timeout. - public void ToolTaskThatTimeoutAndRetry(int repeats, int initialDelay, int followupDelay, int timeout) + [InlineData(1, false)] + [InlineData(3, false)] + [InlineData(3, true)] + public void ToolTaskThatTimeoutAndRetry(int repeats, bool timeoutOnFirstExecution) { using var env = TestEnvironment.Create(_output); + int fastDelayMilliseconds = NativeMethodsShared.IsWindows ? 1_000 : 100; + int slowDelayMilliseconds = 5_000; + int timeoutMilliseconds = NativeMethodsShared.IsWindows ? 2_000 : 1_000; + MockEngine3 engine = new(); // Task under test: var task = new ToolTaskThatSleeps { BuildEngine = engine, - InitialDelay = initialDelay, - FollowupDelay = followupDelay, - Timeout = timeout + InitialDelay = timeoutOnFirstExecution ? slowDelayMilliseconds : fastDelayMilliseconds, + FollowupDelay = fastDelayMilliseconds, + Timeout = timeoutOnFirstExecution ? timeoutMilliseconds : System.Threading.Timeout.Infinite }; // Execute the same task instance multiple times. The index is one-based. - bool result; - for (int i = 1; i <= repeats; i++) + for (int attempt = 1; attempt <= repeats; attempt++) { - // Execute the task: - result = task.Execute(); + bool shouldSucceed = attempt > 1 || !timeoutOnFirstExecution; + bool result = task.Execute(); - _output.WriteLine(engine.Log); + _output.WriteLine( + $"Attempt {attempt}/{repeats}: expectedSuccess={shouldSucceed}, actualSuccess={result}, exitCode={task.ExitCode}."); + + if (!string.IsNullOrEmpty(engine.Log)) + { + _output.WriteLine(engine.Log); + engine.Log = string.Empty; + } - task.RepeatCount.ShouldBe(i); + task.RepeatCount.ShouldBe(attempt); + result.ShouldBe(shouldSucceed); - // The first execution may fail (timeout), but all following ones should succeed: - if (i > 1) + if (shouldSucceed) { - result.ShouldBeTrue(); task.ExitCode.ShouldBe(0); } + else + { + task.ExitCode.ShouldNotBe(0); + } } } From ac443a94ed1c502e6047ad00ed529f20be7c6f88 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 17:03:55 +0000 Subject: [PATCH 3/4] Address review feedback: update doc comment and unify timeoutMilliseconds Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/766be90d-7dd3-492f-bb8d-4bf35765da5b Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com> --- src/Utilities.UnitTests/ToolTask_Tests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Utilities.UnitTests/ToolTask_Tests.cs b/src/Utilities.UnitTests/ToolTask_Tests.cs index eb35569d301..e6c5b091172 100644 --- a/src/Utilities.UnitTests/ToolTask_Tests.cs +++ b/src/Utilities.UnitTests/ToolTask_Tests.cs @@ -996,9 +996,9 @@ public void SetsTerminationTimeoutCorrectly(int timeout, bool isInvalidValid) /// Specifies the number of repeats for external command execution. /// Whether the first execution should be forced to time out before later retries succeed. /// - /// These tests execute the same task instance multiple times using a direct OS sleep process rather than - /// shelling through PowerShell or cmd.exe. When configured to time out, the first execution is expected to - /// fail and all following executions must still succeed. + /// These tests execute the same task instance multiple times, which will in turn run a command to sleep for a + /// predefined amount of time. The first execution may time out, but all following ones won't. It is expected + /// that all following executions return success. /// [Theory] [InlineData(1, false)] @@ -1010,7 +1010,7 @@ public void ToolTaskThatTimeoutAndRetry(int repeats, bool timeoutOnFirstExecutio int fastDelayMilliseconds = NativeMethodsShared.IsWindows ? 1_000 : 100; int slowDelayMilliseconds = 5_000; - int timeoutMilliseconds = NativeMethodsShared.IsWindows ? 2_000 : 1_000; + int timeoutMilliseconds = 2_000; MockEngine3 engine = new(); From dc2f4131dedf318506979242a148bfc2770a4018 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 17:12:31 +0000 Subject: [PATCH 4/4] Use flat fastDelayMilliseconds=100 on all platforms Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/81558234-b82a-4e91-8cbe-75dc53221865 Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com> --- src/Utilities.UnitTests/ToolTask_Tests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utilities.UnitTests/ToolTask_Tests.cs b/src/Utilities.UnitTests/ToolTask_Tests.cs index e6c5b091172..d3df08e1353 100644 --- a/src/Utilities.UnitTests/ToolTask_Tests.cs +++ b/src/Utilities.UnitTests/ToolTask_Tests.cs @@ -1008,7 +1008,7 @@ public void ToolTaskThatTimeoutAndRetry(int repeats, bool timeoutOnFirstExecutio { using var env = TestEnvironment.Create(_output); - int fastDelayMilliseconds = NativeMethodsShared.IsWindows ? 1_000 : 100; + int fastDelayMilliseconds = 100; int slowDelayMilliseconds = 5_000; int timeoutMilliseconds = 2_000;