Skip to content

Commit 6d4a632

Browse files
author
Nate McMaster
committed
Fix race condition in test code waiting for dotnet-watch to restart
1 parent 1f41b26 commit 6d4a632

File tree

4 files changed

+24
-4
lines changed

4 files changed

+24
-4
lines changed

src/dotnet-watch/Internal/ProcessRunner.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,23 @@ public ProcessState(Process process)
105105
{
106106
_process = process;
107107
_process.Exited += OnExited;
108+
Task = _tcs.Task.ContinueWith(_ =>
109+
{
110+
// We need to use two WaitForExit calls to ensure that all of the output/events are processed. Previously
111+
// this code used Process.Exited, which could result in us missing some output due to the ordering of
112+
// events.
113+
//
114+
// See the remarks here: https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.process.waitforexit#System_Diagnostics_Process_WaitForExit_System_Int32_
115+
if (!process.WaitForExit(Int32.MaxValue))
116+
{
117+
throw new TimeoutException();
118+
}
119+
120+
process.WaitForExit();
121+
});
108122
}
109123

110-
public Task Task => _tcs.Task;
124+
public Task Task { get; }
111125

112126
public void TryKill()
113127
{

test/dotnet-watch.FunctionalTests/AwaitableProcess.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ private void OnData(object sender, DataReceivedEventArgs args)
116116

117117
private void OnExit(object sender, EventArgs args)
118118
{
119+
// Wait to ensure the process has exited and all output consumed
120+
_process.WaitForExit();
119121
_source.Complete();
120122
}
121123

test/dotnet-watch.FunctionalTests/NoDepsAppTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public async Task RestartProcessOnFileChange()
4141
Assert.Throws<ArgumentException>(() => Process.GetProcessById(pid));
4242
}
4343

44-
[Fact(Skip="https://github.com/aspnet/DotNetTools/issues/407")]
44+
[Fact]
4545
public async Task RestartProcessThatTerminatesAfterFileChange()
4646
{
4747
await _app.StartWatcherAsync();

test/dotnet-watch.FunctionalTests/Scenario/WatchableApp.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public class WatchableApp : IDisposable
1818

1919
private const string StartedMessage = "Started";
2020
private const string ExitingMessage = "Exiting";
21+
private const string WatchExitedMessage = "watch : Exited";
2122

2223
private readonly ITestOutputHelper _logger;
2324
private string _appName;
@@ -42,8 +43,11 @@ public WatchableApp(string appName, ITestOutputHelper logger)
4243
public Task HasRestarted()
4344
=> Process.GetOutputLineAsync(StartedMessage, DefaultMessageTimeOut);
4445

45-
public Task HasExited()
46-
=> Process.GetOutputLineAsync(ExitingMessage, DefaultMessageTimeOut);
46+
public async Task HasExited()
47+
{
48+
await Process.GetOutputLineAsync(ExitingMessage, DefaultMessageTimeOut);
49+
await Process.GetOutputLineAsync(WatchExitedMessage, DefaultMessageTimeOut);
50+
}
4751

4852
public bool UsePollingWatcher { get; set; }
4953

0 commit comments

Comments
 (0)