-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Stabilize ToolTaskThatTimeoutAndRetry test #13489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rainersigwald
wants to merge
4
commits into
main
Choose a base branch
from
tooltask-timeout-retry-test
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+56
−44
Open
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
0b1f521
Use ping.exe in ToolTask sleep helper
rainersigwald 47ad858
Refactor ToolTask timeout retry test
rainersigwald ac443a9
Address review feedback: update doc comment and unify timeoutMillisec…
Copilot dc2f413
Use flat fastDelayMilliseconds=100 on all platforms
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
|
|
||
| using System; | ||
| using System.Diagnostics; | ||
| using System.Globalization; | ||
| using System.IO; | ||
| using System.Resources; | ||
| using System.Text.RegularExpressions; | ||
|
|
@@ -20,7 +21,7 @@ namespace Microsoft.Build.UnitTests | |
| { | ||
| public sealed class ToolTask_Tests | ||
| { | ||
| private ITestOutputHelper _output; | ||
| private readonly ITestOutputHelper _output; | ||
|
|
||
| public ToolTask_Tests(ITestOutputHelper testOutput) | ||
| { | ||
|
|
@@ -993,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. | ||
| /// </summary> | ||
| /// <param name="repeats">Specifies the number of repeats for external command execution.</param> | ||
| /// <param name="initialDelay">Delay to generate on the first execution in milliseconds.</param> | ||
| /// <param name="followupDelay">Delay to generate on follow-up execution in milliseconds.</param> | ||
| /// <param name="timeout">Task timeout in milliseconds.</param> | ||
| /// <param name="timeoutOnFirstExecution">Whether the first execution should be forced to time out before later retries succeed.</param> | ||
| /// <remarks> | ||
| /// These tests execute the same task instance multiple times, which will in turn run a shell command to sleep | ||
| /// 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. | ||
| /// </remarks> | ||
| [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 = 2_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); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1106,25 +1118,21 @@ public void ToolTaskCapturesAllOutputWithFix() | |
| /// A simple implementation of <see cref="ToolTask"/> to sleep for a while. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// 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. | ||
| /// </remarks> | ||
| 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"); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -1139,9 +1147,9 @@ public ToolTaskThatSleeps() | |
| /// Gets or sets the delay for the follow-up executions. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Defaults to 1 milliseconds. | ||
| /// Defaults to 100 milliseconds. | ||
| /// </remarks> | ||
| public Int32 FollowupDelay { get; set; } = 1; | ||
| public Int32 FollowupDelay { get; set; } = 100; | ||
|
|
||
| /// <summary> | ||
| /// Int32 output parameter for the repeat counter for test purpose. | ||
|
|
@@ -1150,22 +1158,26 @@ public ToolTaskThatSleeps() | |
| public Int32 RepeatCount { get; private set; } = 0; | ||
|
|
||
| /// <summary> | ||
| /// Gets the tool name (shell). | ||
| /// Gets the tool name. | ||
| /// </summary> | ||
| protected override string ToolName => Path.GetFileName(_pathToShell); | ||
| protected override string ToolName => Path.GetFileName(_pathToTool); | ||
|
|
||
| /// <summary> | ||
| /// Gets the full path to shell. | ||
| /// Gets the full path to the sleep tool. | ||
| /// </summary> | ||
| protected override string GenerateFullPathToTool() => _pathToShell; | ||
| protected override string GenerateFullPathToTool() => _pathToTool; | ||
|
|
||
| /// <summary> | ||
| /// 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. | ||
| /// </summary> | ||
| 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); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hilariously, before this change we tried to
|
||
| 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"; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Ensures that test parameters make sense. | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
start /waitwas (at least part of!) the reason running tests in our repo popped up terminal windows over and over. Not sure if that was bugging anybody else but it drove me up the wall!