From b65e3c67292eb5becd3cc713e8fec5fcd8db40ad Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Thu, 19 Dec 2024 15:21:46 -0800 Subject: [PATCH 1/3] Test fixes --- .../HotReload/ApplyDeltaTests.cs | 37 ++++++++++--------- .../Watch/Utilities/WatchableApp.cs | 1 + 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index ba1ca06620b5..602dce920d46 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -248,7 +248,7 @@ class AppUpdateHandler await App.AssertOutputLineStartsWith("Updated"); await App.WaitUntilOutputContains( - "dotnet watch ⚠ [WatchHotReloadApp (net9.0)] Expected to find a static method 'ClearCache' or 'UpdateApplication' on type 'AppUpdateHandler, WatchHotReloadApp, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' but neither exists."); + $"dotnet watch ⚠ [WatchHotReloadApp ({ToolsetInfo.CurrentTargetFramework})] Expected to find a static method 'ClearCache' or 'UpdateApplication' on type 'AppUpdateHandler, WatchHotReloadApp, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' but neither exists."); } [Theory] @@ -287,11 +287,11 @@ class AppUpdateHandler await App.AssertOutputLineStartsWith("Updated"); - await App.WaitUntilOutputContains("dotnet watch ⚠ [WatchHotReloadApp (net9.0)] Exception from 'System.Action`1[System.Type[]]': System.InvalidOperationException: Bug!"); + await App.WaitUntilOutputContains($"dotnet watch ⚠ [WatchHotReloadApp ({ToolsetInfo.CurrentTargetFramework})] Exception from 'System.Action`1[System.Type[]]': System.InvalidOperationException: Bug!"); if (verbose) { - await App.WaitUntilOutputContains("dotnet watch 🕵️ [WatchHotReloadApp (net9.0)] Deltas applied."); + await App.WaitUntilOutputContains($"dotnet watch 🕵️ [WatchHotReloadApp ({ToolsetInfo.CurrentTargetFramework})] Deltas applied."); } else { @@ -347,7 +347,7 @@ public async Task BlazorWasm(bool projectSpecifiesCapabilities) """; UpdateSourceFile(Path.Combine(testAsset.Path, "Pages", "Index.razor"), newSource); - await App.AssertOutputLineStartsWith(MessageDescriptor.HotReloadSucceeded, "blazorwasm (net9.0)"); + await App.AssertOutputLineStartsWith(MessageDescriptor.HotReloadSucceeded, $"blazorwasm ({ToolsetInfo.CurrentTargetFramework})"); // check project specified capapabilities: if (projectSpecifiesCapabilities) @@ -410,8 +410,8 @@ public async Task Razor_Component_ScopedCssAndStaticAssets() await App.AssertOutputLineStartsWith("dotnet watch 🔥 Hot reload change handled"); App.AssertOutputContains($"dotnet watch ⌚ Handling file change event for scoped css file {scopedCssPath}."); - App.AssertOutputContains($"dotnet watch ⌚ [RazorClassLibrary (net9.0)] No refresh server."); - App.AssertOutputContains($"dotnet watch ⌚ [RazorApp (net9.0)] Refreshing browser."); + App.AssertOutputContains($"dotnet watch ⌚ [RazorClassLibrary ({ToolsetInfo.CurrentTargetFramework})] No refresh server."); + App.AssertOutputContains($"dotnet watch ⌚ [RazorApp ({ToolsetInfo.CurrentTargetFramework})] Refreshing browser."); App.AssertOutputContains($"dotnet watch 🔥 Hot reload of scoped css succeeded."); App.AssertOutputContains($"dotnet watch ⌚ No C# changes to apply."); App.Process.ClearOutput(); @@ -422,7 +422,7 @@ public async Task Razor_Component_ScopedCssAndStaticAssets() await App.AssertOutputLineStartsWith("dotnet watch 🔥 Hot reload change handled"); App.AssertOutputContains($"dotnet watch ⌚ Sending static file update request for asset 'app.css'."); - App.AssertOutputContains($"dotnet watch ⌚ [RazorApp (net9.0)] Refreshing browser."); + App.AssertOutputContains($"dotnet watch ⌚ [RazorApp ({ToolsetInfo.CurrentTargetFramework})] Refreshing browser."); App.AssertOutputContains($"dotnet watch 🔥 Hot Reload of static files succeeded."); App.AssertOutputContains($"dotnet watch ⌚ No C# changes to apply."); App.Process.ClearOutput(); @@ -620,6 +620,7 @@ public static void PrintDirectoryName([CallerFilePathAttribute] string filePath [Fact] public async Task Aspire() { + var tfm = ToolsetInfo.CurrentTargetFramework; var testAsset = TestAssets.CopyTestAsset("WatchAspire") .WithSource(); @@ -645,8 +646,8 @@ public async Task Aspire() await App.AssertOutputLineStartsWith("dotnet watch 🔥 Hot reload change handled"); App.AssertOutputContains("Using Aspire process launcher."); - App.AssertOutputContains(MessageDescriptor.HotReloadSucceeded, "WatchAspire.AppHost (net9.0)"); - App.AssertOutputContains(MessageDescriptor.HotReloadSucceeded, "WatchAspire.ApiService (net9.0)"); + App.AssertOutputContains(MessageDescriptor.HotReloadSucceeded, $"WatchAspire.AppHost ({tfm})"); + App.AssertOutputContains(MessageDescriptor.HotReloadSucceeded, $"WatchAspire.ApiService ({tfm})"); // Only one browser should be launched (dashboard). The child process shouldn't launch a browser. Assert.Equal(1, App.Process.Output.Count(line => line.StartsWith("dotnet watch ⌚ Launching browser: "))); @@ -672,13 +673,13 @@ public async Task Aspire() // We don't have means to gracefully terminate process on Windows, see https://github.com/dotnet/runtime/issues/109432 if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - App.AssertOutputContains("dotnet watch ❌ [WatchAspire.ApiService (net9.0)] Exited with error code -1"); + App.AssertOutputContains($"dotnet watch ❌ [WatchAspire.ApiService ({tfm})] Exited with error code -1"); } else { // Unix process may return exit code = 128 + SIGTERM - // dotnet watch ❌ [WatchAspire.ApiService (net9.0)] Exited with error code 143 - App.AssertOutputContains("[WatchAspire.ApiService (net9.0)] Exited"); + // Exited with error code 143 + App.AssertOutputContains($"[WatchAspire.ApiService ({tfm})] Exited"); } App.AssertOutputContains($"dotnet watch ⌚ Building '{serviceProjectPath}' ..."); @@ -690,7 +691,7 @@ public async Task Aspire() serviceSourcePath, originalSource.Replace("WeatherForecast", "WeatherForecast2")); - await App.AssertOutputLineStartsWith("dotnet watch ⌚ [WatchAspire.ApiService (net9.0)] Capabilities"); + await App.AssertOutputLineStartsWith($"dotnet watch ⌚ [WatchAspire.ApiService ({tfm})] Capabilities"); App.AssertOutputContains("dotnet watch ⌚ Build succeeded."); App.AssertOutputContains("dotnet watch 🔥 Project baselines updated."); @@ -703,15 +704,15 @@ public async Task Aspire() // We don't have means to gracefully terminate process on Windows, see https://github.com/dotnet/runtime/issues/109432 if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - await App.AssertOutputLineStartsWith("dotnet watch ❌ [WatchAspire.ApiService (net9.0)] Exited with error code -1"); - await App.AssertOutputLineStartsWith("dotnet watch ❌ [WatchAspire.AppHost (net9.0)] Exited with error code -1"); + await App.AssertOutputLineStartsWith($"dotnet watch ❌ [WatchAspire.ApiService ({tfm})] Exited with error code -1"); + await App.AssertOutputLineStartsWith($"dotnet watch ❌ [WatchAspire.AppHost ({tfm})] Exited with error code -1"); } else { // Unix process may return exit code = 128 + SIGTERM - // dotnet watch ❌ [WatchAspire.ApiService (net9.0)] Exited with error code 143 - await App.AssertOutputLine(line => line.Contains("[WatchAspire.ApiService (net9.0)] Exited"), failure: _ => false); - await App.AssertOutputLine(line => line.Contains("[WatchAspire.AppHost (net9.0)] Exited"), failure: _ => false); + // Exited with error code 143 + await App.AssertOutputLine(line => line.Contains($"[WatchAspire.ApiService ({tfm})] Exited"), failure: _ => false); + await App.AssertOutputLine(line => line.Contains($"[WatchAspire.AppHost ({tfm})] Exited"), failure: _ => false); } await App.AssertOutputLineStartsWith("dotnet watch ⭐ Waiting for server to shutdown ..."); diff --git a/test/dotnet-watch.Tests/Watch/Utilities/WatchableApp.cs b/test/dotnet-watch.Tests/Watch/Utilities/WatchableApp.cs index c949b46140bd..d1476b64c869 100644 --- a/test/dotnet-watch.Tests/Watch/Utilities/WatchableApp.cs +++ b/test/dotnet-watch.Tests/Watch/Utilities/WatchableApp.cs @@ -42,6 +42,7 @@ public async ValueTask WaitUntilOutputContains(string message) { if (!Process.Output.Any(line => line.Contains(message))) { + Logger.WriteLine($"[TEST] Test waiting for output: '{message}'"); _ = await AssertOutputLine(line => line.Contains(message)); } } From c7e93f0bd57b190663fee422d5a90711038741c3 Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Thu, 19 Dec 2024 17:06:38 -0800 Subject: [PATCH 2/3] Improve output reporting, fix potential race condition of output redirection --- .../dotnet-watch/HotReloadDotNetWatcher.cs | 26 +++++++++---------- .../Internal/MsBuildFileSetFactory.cs | 2 +- .../dotnet-watch/Internal/ProcessRunner.cs | 9 +++++++ .../Properties/launchSettings.json | 2 +- .../dotnet-watch/Utilities/BuildUtilities.cs | 17 +++++++++--- .../HotReload/ApplyDeltaTests.cs | 4 +-- .../Utilities/AwaitableProcess.cs | 2 +- 7 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs b/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs index e0f7ae1115f0..4402c1fbf742 100644 --- a/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs @@ -121,9 +121,10 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke }; } - if (!await BuildProjectAsync(rootProjectOptions.ProjectPath, rootProjectOptions.BuildArguments, iterationCancellationToken)) + var (buildSucceeded, buildOutput, _) = await BuildProjectAsync(rootProjectOptions.ProjectPath, rootProjectOptions.BuildArguments, iterationCancellationToken); + BuildUtilities.ReportBuildOutput(Context.Reporter, buildOutput, buildSucceeded, projectDisplay: rootProjectOptions.ProjectPath); + if (!buildSucceeded) { - // error has been reported: continue; } @@ -334,7 +335,12 @@ void FileChangedCallback(ChangedPath change) var buildResults = await Task.WhenAll( projectsToRebuild.Values.Select(projectPath => BuildProjectAsync(projectPath, rootProjectOptions.BuildArguments, iterationCancellationToken))); - if (buildResults.All(success => success)) + foreach (var (success, output, projectPath) in buildResults) + { + BuildUtilities.ReportBuildOutput(Context.Reporter, output, success, projectPath); + } + + if (buildResults.All(result => result.success)) { break; } @@ -815,7 +821,8 @@ await FileWatcher.WaitForFileChangeAsync( } } - private async Task BuildProjectAsync(string projectPath, IReadOnlyList buildArguments, CancellationToken cancellationToken) + private async Task<(bool success, ImmutableArray output, string projectPath)> BuildProjectAsync( + string projectPath, IReadOnlyList buildArguments, CancellationToken cancellationToken) { var buildOutput = new List(); @@ -834,17 +841,10 @@ private async Task BuildProjectAsync(string projectPath, IReadOnlyList RunAsync(ProcessSpec processSpec, IReporter report try { await process.WaitForExitAsync(processTerminationToken); + + // ensures that all process output has been reported: + try + { + process.WaitForExit(); + } + catch + { + } } catch (OperationCanceledException) { diff --git a/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json b/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json index de41cecc021a..9f3b0a151069 100644 --- a/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json +++ b/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json @@ -3,7 +3,7 @@ "dotnet-watch": { "commandName": "Project", "commandLineArgs": "--verbose /bl:DotnetRun.binlog", - "workingDirectory": "$(RepoRoot)src\\Assets\\TestProjects\\BlazorWasmWithLibrary\\blazorwasm", + "workingDirectory": "C:\\sdk5\\artifacts\\tmp\\Debug\\Aspire_ApplyD---C6DC4E42\\WatchAspire.AppHost", "environmentVariables": { "DOTNET_WATCH_DEBUG_SDK_DIRECTORY": "$(RepoRoot)artifacts\\bin\\redist\\$(Configuration)\\dotnet\\sdk\\$(Version)", "DCP_IDE_REQUEST_TIMEOUT_SECONDS": "100000", diff --git a/src/BuiltInTools/dotnet-watch/Utilities/BuildUtilities.cs b/src/BuiltInTools/dotnet-watch/Utilities/BuildUtilities.cs index 3c9d85e0be4b..291402fc337a 100644 --- a/src/BuiltInTools/dotnet-watch/Utilities/BuildUtilities.cs +++ b/src/BuiltInTools/dotnet-watch/Utilities/BuildUtilities.cs @@ -7,14 +7,25 @@ namespace Microsoft.DotNet.Watch; internal static partial class BuildUtilities { + private const string BuildEmoji = "🔨"; private static readonly Regex s_buildDiagnosticRegex = GetBuildDiagnosticRegex(); [GeneratedRegex(@"[^:]+: (error|warning) [A-Za-z]+[0-9]+: .+")] private static partial Regex GetBuildDiagnosticRegex(); - public static void ReportBuildOutput(IReporter reporter, IEnumerable buildOutput, bool verboseOutput) + public static void ReportBuildOutput(IReporter reporter, IEnumerable buildOutput, bool success, string? projectDisplay) { - const string BuildEmoji = "🔨"; + if (projectDisplay != null) + { + if (success) + { + reporter.Output($"Build succeeded: {projectDisplay}", BuildEmoji); + } + else + { + reporter.Output($"Build failed: {projectDisplay}", BuildEmoji); + } + } foreach (var (line, isError) in buildOutput) { @@ -33,7 +44,7 @@ public static void ReportBuildOutput(IReporter reporter, IEnumerable reporter.Warn(line); } } - else if (verboseOutput) + else if (success) { reporter.Verbose(line, BuildEmoji); } diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index 602dce920d46..c73380b7f4b8 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -682,7 +682,7 @@ public async Task Aspire() App.AssertOutputContains($"[WatchAspire.ApiService ({tfm})] Exited"); } - App.AssertOutputContains($"dotnet watch ⌚ Building '{serviceProjectPath}' ..."); + App.AssertOutputContains($"dotnet watch ⌚ Building {serviceProjectPath} ..."); App.AssertOutputContains("error CS0246: The type or namespace name 'WeatherForecast' could not be found"); App.Process.ClearOutput(); @@ -693,7 +693,7 @@ public async Task Aspire() await App.AssertOutputLineStartsWith($"dotnet watch ⌚ [WatchAspire.ApiService ({tfm})] Capabilities"); - App.AssertOutputContains("dotnet watch ⌚ Build succeeded."); + App.AssertOutputContains($"dotnet watch 🔨 Build succeeded: {serviceProjectPath}"); App.AssertOutputContains("dotnet watch 🔥 Project baselines updated."); App.AssertOutputContains($"dotnet watch ⭐ Starting project: {serviceProjectPath}"); diff --git a/test/dotnet-watch.Tests/Utilities/AwaitableProcess.cs b/test/dotnet-watch.Tests/Utilities/AwaitableProcess.cs index 757bf46d20c1..369f875f031d 100644 --- a/test/dotnet-watch.Tests/Utilities/AwaitableProcess.cs +++ b/test/dotnet-watch.Tests/Utilities/AwaitableProcess.cs @@ -129,7 +129,7 @@ private void OnData(object sender, DataReceivedEventArgs args) line = line.StripTerminalLoggerProgressIndicators(); } - WriteTestOutput($"{DateTime.Now}: post: '{line}'"); + WriteTestOutput(line); _source.Post(line); } From 2af79abde74e23078eb181628ce7c352b3e17cf8 Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Fri, 20 Dec 2024 07:59:34 -0800 Subject: [PATCH 3/3] Revert launchSettings --- src/BuiltInTools/dotnet-watch/Properties/launchSettings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json b/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json index 9f3b0a151069..de41cecc021a 100644 --- a/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json +++ b/src/BuiltInTools/dotnet-watch/Properties/launchSettings.json @@ -3,7 +3,7 @@ "dotnet-watch": { "commandName": "Project", "commandLineArgs": "--verbose /bl:DotnetRun.binlog", - "workingDirectory": "C:\\sdk5\\artifacts\\tmp\\Debug\\Aspire_ApplyD---C6DC4E42\\WatchAspire.AppHost", + "workingDirectory": "$(RepoRoot)src\\Assets\\TestProjects\\BlazorWasmWithLibrary\\blazorwasm", "environmentVariables": { "DOTNET_WATCH_DEBUG_SDK_DIRECTORY": "$(RepoRoot)artifacts\\bin\\redist\\$(Configuration)\\dotnet\\sdk\\$(Version)", "DCP_IDE_REQUEST_TIMEOUT_SECONDS": "100000",