Remove blocking call to stream reader and fix bug in task chaining

build/repo.props

@@ -1,5 +0,0 @@
-<Project>
-  <ItemGroup>
-  	<ExcludeFromTest Include="$(RepositoryRoot)test\Microsoft.DotNet.Watcher.Tools.FunctionalTests\*.csproj" />
-  </ItemGroup>
-</Project>

test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/AppWithDepsTests.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -16,7 +16,6 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
         public AppWithDepsTests(ITestOutputHelper logger)
         {
             _app = new AppWithDeps(logger);
-            _app.Prepare();
         }
 
         [Fact]

test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/AwaitableProcess.cs

@@ -4,9 +4,6 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.IO;
-using System.Text;
-using System.Threading;
 using System.Threading.Tasks;
 using System.Threading.Tasks.Dataflow;
 using Microsoft.Extensions.Internal;
@@ -21,12 +18,12 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
         private readonly ProcessSpec _spec;
         private BufferBlock<string> _source;
         private ITestOutputHelper _logger;
-        private int _reading;
 
         public AwaitableProcess(ProcessSpec spec, ITestOutputHelper logger)
         {
             _spec = spec;
             _logger = logger;
+            _source = new BufferBlock<string>();
         }
 
         public void Start()
@@ -36,19 +33,32 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
                 throw new InvalidOperationException("Already started");
             }
 
-            var psi = new ProcessStartInfo
+            _process = new Process
             {
-                UseShellExecute = false,
+                EnableRaisingEvents = true,
-                FileName = _spec.Executable,
+                StartInfo = new ProcessStartInfo
-                WorkingDirectory = _spec.WorkingDirectory,
+                {
-                Arguments = ArgumentEscaper.EscapeAndConcatenate(_spec.Arguments),
+                    UseShellExecute = false,
-                RedirectStandardOutput = true,
+                    FileName = _spec.Executable,
-                RedirectStandardError = true
+                    WorkingDirectory = _spec.WorkingDirectory,
+                    Arguments = ArgumentEscaper.EscapeAndConcatenate(_spec.Arguments),
+                    RedirectStandardOutput = true,
+                    RedirectStandardError = true,
+                    Environment =
+                    {
+                        ["DOTNET_SKIP_FIRST_TIME_EXPERIENCE"] = "true"
+                    }
+                }
             };
-            _process = Process.Start(psi);
+
-            _logger.WriteLine($"{DateTime.Now}: process start: '{psi.FileName} {psi.Arguments}'");
+            _process.OutputDataReceived += OnData;
-            StartProcessingOutput(_process.StandardOutput);
+            _process.ErrorDataReceived += OnData;
-            StartProcessingOutput(_process.StandardError);;
+            _process.Exited += OnExit;
+
+            _process.Start();
+            _process.BeginErrorReadLine();
+            _process.BeginOutputReadLine();
+            _logger.WriteLine($"{DateTime.Now}: process start: '{_process.StartInfo.FileName} {_process.StartInfo.Arguments}'");
         }
 
         public Task<string> GetOutputLineAsync(string message)
@@ -87,32 +97,33 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
             return lines;
         }
 
-        private void StartProcessingOutput(StreamReader streamReader)
+        private void OnData(object sender, DataReceivedEventArgs args)
         {
-            _source = _source ?? new BufferBlock<string>();
+            var line = args.Data ?? string.Empty;
-            Interlocked.Increment(ref _reading);
+            _logger.WriteLine($"{DateTime.Now}: post: '{line}'");
-            Task.Run(() =>
+            _source.Post(line);
-            {
+        }
-                string line;
-                while ((line = streamReader.ReadLine()) != null)
-                {
-                    _logger.WriteLine($"{DateTime.Now}: post: '{line}'");
-                    _source.Post(line);
-                }
 
-                if (Interlocked.Decrement(ref _reading) <= 0)
+        private void OnExit(object sender, EventArgs args)
-                {
+        {
-                    _source.Complete();
+            _source.Complete();
-                }
-            }).ConfigureAwait(false);
         }
 
         public void Dispose()
         {
-            if (_process != null && !_process.HasExited)
+            _source.Complete();
+
+            if (_process != null)
             {
-                _process.KillTree();
+                if (!_process.HasExited)
+                {
+                    _process.KillTree();
+                }
+
+                _process.ErrorDataReceived -= OnData;
+                _process.OutputDataReceived -= OnData;
+                _process.Exited -= OnExit;
             }
         }
     }
-}
+}

test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/GlobbingAppTests.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -17,7 +17,6 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
         public GlobbingAppTests(ITestOutputHelper logger)
         {
             _app = new GlobbingApp(logger);
-            _app.Prepare();
         }
 
         [Theory]
@@ -115,6 +114,7 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
         [Fact]
         public async Task ListsFiles()
         {
+            await _app.PrepareAsync();
             _app.Start(new [] { "--list" });
             var lines = await _app.Process.GetAllOutputLines();
 

test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/NoDepsAppTests.cs

@@ -17,7 +17,6 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
         public NoDepsAppTests(ITestOutputHelper logger)
         {
             _app = new WatchableApp("NoDepsApp", logger);
-            _app.Prepare();
         }
 
         [Fact]

test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Scenario/ProjectToolScenario.cs

@@ -1,13 +1,15 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using Microsoft.Extensions.Tools.Internal;
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.Reflection;
 using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.Extensions.Internal;
+using Microsoft.Extensions.Tools.Internal;
 using Xunit.Abstractions;
 
 namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
@@ -55,50 +57,84 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
             }
         }
 
-        public void Restore(string project)
+        public Task RestoreAsync(string project)
         {
             _logger?.WriteLine($"Restoring msbuild project in {project}");
-            ExecuteCommand(project, "restore");
+            return ExecuteCommandAsync(project, TimeSpan.FromSeconds(120), "restore");
         }
 
-        public void Build(string project)
+        public Task BuildAsync(string project)
         {
             _logger?.WriteLine($"Building {project}");
-            ExecuteCommand(project, "build");
+            return ExecuteCommandAsync(project, TimeSpan.FromSeconds(60), "build");
         }
 
-        private void ExecuteCommand(string project, params string[] arguments)
+        private async Task ExecuteCommandAsync(string project, TimeSpan timeout, params string[] arguments)
         {
+            var tcs = new TaskCompletionSource<object>();
             project = Path.Combine(WorkFolder, project);
-            var psi = new ProcessStartInfo
+            _logger?.WriteLine($"Project directory: '{project}'");
+
+            var process = new Process
             {
-                FileName = DotNetMuxer.MuxerPathOrDefault(),
+                EnableRaisingEvents = true,
-                Arguments = ArgumentEscaper.EscapeAndConcatenate(arguments),
+                StartInfo = new ProcessStartInfo
-                WorkingDirectory = project,
+                {
-                RedirectStandardOutput = true,
+                    FileName = DotNetMuxer.MuxerPathOrDefault(),
-                RedirectStandardError = true
+                    Arguments = ArgumentEscaper.EscapeAndConcatenate(arguments),
+                    WorkingDirectory = project,
+                    RedirectStandardOutput = true,
+                    RedirectStandardError = true,
+                    Environment =
+                    {
+                        ["DOTNET_SKIP_FIRST_TIME_EXPERIENCE"] = "true"
+                    }
+                },
             };
 
-            var process = new Process()
+            void OnData(object sender, DataReceivedEventArgs args)
+              => _logger?.WriteLine(args.Data ?? string.Empty);
+
+            void OnExit(object sender, EventArgs args)
             {
-                StartInfo = psi,
+                _logger?.WriteLine($"Process exited {process.Id}");
-                EnableRaisingEvents = true
+                tcs.TrySetResult(null);
-            };
+            }
 
-            void WriteLine(object sender, DataReceivedEventArgs args)
+            process.ErrorDataReceived += OnData;
-              => _logger.WriteLine(args.Data);
+            process.OutputDataReceived += OnData;
-
+            process.Exited += OnExit;
-            process.ErrorDataReceived += WriteLine;
-            process.OutputDataReceived += WriteLine;
 
             process.Start();
-            process.WaitForExit();
 
-            process.ErrorDataReceived -= WriteLine;
+            process.BeginErrorReadLine();
-            process.OutputDataReceived -= WriteLine;
+            process.BeginOutputReadLine();
 
+            _logger?.WriteLine($"Started process {process.Id}: {process.StartInfo.FileName} {process.StartInfo.Arguments}");
+
+            var done = await Task.WhenAny(tcs.Task, Task.Delay(timeout));
+            process.CancelErrorRead();
+            process.CancelOutputRead();
+
+            process.ErrorDataReceived -= OnData;
+            process.OutputDataReceived -= OnData;
+            process.Exited -= OnExit;
+
+            if (!ReferenceEquals(done, tcs.Task))
+            {
+                if (!process.HasExited)
+                {
+                    _logger?.WriteLine($"Killing process {process.Id}");
+                    process.KillTree();
+                }
+
+                throw new TimeoutException($"Process timed out after {timeout.TotalSeconds} seconds");
+            }
+
+            _logger?.WriteLine($"Process exited {process.Id} with code {process.ExitCode}");
             if (process.ExitCode != 0)
             {
+
                 throw new InvalidOperationException($"Exit code {process.ExitCode}");
             }
         }
@@ -179,4 +215,4 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
             }
         }
     }
-}
+}

test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Scenario/WatchableApp.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -52,10 +52,10 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
             return int.Parse(pid);
         }
 
-        public void Prepare()
+        public async Task PrepareAsync()
         {
-            Scenario.Restore(_appName);
+            await Scenario.RestoreAsync(_appName);
-            Scenario.Build(_appName);
+            await Scenario.BuildAsync(_appName);
             _prepared = true;
         }
 
@@ -63,7 +63,7 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
         {
             if (!_prepared)
             {
-                throw new InvalidOperationException("Call .Prepare() first");
+                throw new InvalidOperationException($"Call {nameof(PrepareAsync)} first");
             }
 
             var args = Scenario
@@ -86,6 +86,11 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
 
         public async Task StartWatcherAsync(string[] arguments, [CallerMemberName] string name = null)
         {
+            if (!_prepared)
+            {
+                await PrepareAsync();
+            }
+
             var args = new[] { "run", "--" }.Concat(arguments);
             Start(args, name);
             await Process.GetOutputLineAsync(StartedMessage);
@@ -93,8 +98,8 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests
 
         public virtual void Dispose()
         {
-            Process.Dispose();
+            Process?.Dispose();
-            Scenario.Dispose();
+            Scenario?.Dispose();
         }
     }
 }

test/Microsoft.DotNet.Watcher.Tools.Tests/AssertEx.cs

@@ -21,7 +21,7 @@ namespace Microsoft.DotNet.Watcher.Tools.Tests
         {
             Func<string, string> normalize = p => p.Replace('\\', '/');
             var expected = new HashSet<string>(expectedFiles.Select(normalize));
-            Assert.True(expected.SetEquals(actualFiles.Select(normalize)), "File sets should be equal");
+            Assert.True(expected.SetEquals(actualFiles.Where(p => !string.IsNullOrEmpty(p)).Select(normalize)), "File sets should be equal");
         }
     }
-}
+}

test/Microsoft.DotNet.Watcher.Tools.Tests/Utilities/TaskExtensions.cs

@@ -9,8 +9,12 @@ namespace System.Threading.Tasks
     {
         public static async Task<T> OrTimeout<T>(this Task<T> task, int timeout = 30, [CallerFilePath] string file = null, [CallerLineNumber] int line = 0)
         {
-            await OrTimeout((Task)task, timeout, file, line);
+            var finished = await Task.WhenAny(task, Task.Delay(TimeSpan.FromSeconds(timeout)));
-            return task.Result;
+            if (!ReferenceEquals(finished, task))
+            {
+                throw new TimeoutException($"Task exceeded max running time of {timeout}s at {file}:{line}");
+            }
+            return await task;
         }
 
         public static async Task OrTimeout(this Task task, int timeout = 30, [CallerFilePath] string file = null, [CallerLineNumber] int line = 0)
@@ -20,6 +24,7 @@ namespace System.Threading.Tasks
             {
                 throw new TimeoutException($"Task exceeded max running time of {timeout}s at {file}:{line}");
             }
+            await task;
         }
     }
-}
+}