Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 131 additions & 1 deletion src/Utilities.UnitTests/ToolTask_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ protected override int ExecuteTool(string pathToTool, string responseFileCommand
StartInfo = GetProcessStartInfo(GenerateFullPathToTool(), NativeMethodsShared.IsWindows ? "/x" : string.Empty, null);
return result;
}
};
}

[Fact]
public void Regress_Mutation_UserSuppliedToolPathIsLogged()
Expand Down Expand Up @@ -825,5 +825,135 @@ protected override string GenerateCommandLineCommands()
return $"echo łoł > {OutputPath}";
}
}

/// <summary>
/// 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>
/// <remarks>
/// 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.
/// </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)
{
using var env = TestEnvironment.Create(_output);

// Task under test:
var task = new ToolTaskThatSleeps
{
BuildEngine = new MockEngine(),
InitialDelay = initialDelay,
FollowupDelay = followupDelay,
Timeout = timeout
};

// Execute the same task instance multiple times. The index is one-based.
bool result;
for (int i = 1; i <= repeats; i++)
{
// Execute the task:
result = task.Execute();
task.RepeatCount.ShouldBe(i);

// The first execution may fail (timeout), but all following ones should succeed:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In tests the ideal timeouts are infinitely large so they never hit or infinitely small (1ms) so they always hit or will only hit if the test discovers a bug. Is that possible?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per #8546, the timeout isn't properly used at the moment. We could consider delaying this PR until the other one is in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In tests the ideal timeouts are infinitely large so they never hit or infinitely small (1ms) so they always hit or will only hit if the test discovers a bug. Is that possible?

Hi @danmoseley, the test case will run sleep command to create some delay. It's possible to let the command sleep infinitely, it's controlled by XUnit inline parameters. However, if Timeout failed when infinite delay is in use, the test will hang forever. So I set that to 10s, with a timeout of 1s, just to be safe. Yes, there's certain possibility that the timeout won't play out as planned. Just some tradeoff.

Do you think I should set that delay in case 3 to a larger number (like 1 hour)?

if (i > 1)
{
result.ShouldBeTrue();
task.ExitCode.ShouldBe(0);
}
}
}

/// <summary>
/// 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.
/// </remarks>
private sealed class ToolTaskThatSleeps : ToolTask
{
// PowerShell command to sleep:
private readonly string _powerShellSleep = "-ExecutionPolicy RemoteSigned -Command \"Start-Sleep -Milliseconds {0}\"";

// UNIX command to sleep:
private readonly string _unixSleep = "-c \"sleep {0}\"";

// Full path to shell:
private readonly string _pathToShell;

public ToolTaskThatSleeps()
: base()
{
// Determines shell to use: PowerShell for Windows, sh for UNIX-like systems:
_pathToShell = NativeMethodsShared.IsUnixLike ? "/bin/sh" : FindOnPath("PowerShell.exe");
}

/// <summary>
/// Gets or sets the delay for the first execution.
/// </summary>
/// <remarks>
/// Defaults to 10 seconds.
/// </remarks>
public Int32 InitialDelay { get; set; } = 10000;

/// <summary>
/// Gets or sets the delay for the follow-up executions.
/// </summary>
/// <remarks>
/// Defaults to 1 milliseconds.
/// </remarks>
public Int32 FollowupDelay { get; set; } = 1;

/// <summary>
/// Int32 output parameter for the repeat counter for test purpose.
/// </summary>
[Output]
public Int32 RepeatCount { get; private set; } = 0;

/// <summary>
/// Gets the tool name (shell).
/// </summary>
protected override string ToolName => Path.GetFileName(_pathToShell);

/// <summary>
/// Gets the full path to shell.
/// </summary>
protected override string GenerateFullPathToTool() => _pathToShell;

/// <summary>
/// Generates a shell command to sleep 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(_powerShellSleep, RepeatCount < 2 ? InitialDelay : FollowupDelay);

/// <summary>
/// Ensures that test parameters make sense.
/// </summary>
protected internal override bool ValidateParameters() =>
(InitialDelay > 0) && (FollowupDelay > 0) && base.ValidateParameters();

/// <summary>
/// Runs shell command to sleep for a while.
/// </summary>
/// <returns>
/// true if the task runs successfully; false otherwise.
/// </returns>
public override bool Execute()
{
RepeatCount++;
return base.Execute();
}
}
}
}
1 change: 1 addition & 0 deletions src/Utilities/ToolTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,7 @@ protected virtual int ExecuteTool(
_standardOutputDataAvailable = new ManualResetEvent(false);

_toolExited = new ManualResetEvent(false);
_terminatedTool = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other things that need to be reset that currently aren't being reset? I'm wondering if we should move this to a separate "ResetToolTaskState" method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked all related working variables (fields), I think all of them should be handled for now.

_toolTimeoutExpired = new ManualResetEvent(false);

_eventsDisposed = false;
Expand Down