Skip to content
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

Synchronize watched process and reporter output printing #46141

Merged
merged 2 commits into from
Jan 24, 2025
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
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Build.Graph;

namespace Microsoft.DotNet.Watch;

internal sealed class BrowserSpecificReporter(int browserId, IReporter underlyingReporter) : IReporter
Expand All @@ -12,14 +10,8 @@ internal sealed class BrowserSpecificReporter(int browserId, IReporter underlyin
public bool IsVerbose
=> underlyingReporter.IsVerbose;

public bool EnableProcessOutputReporting
=> false;

public void ReportProcessOutput(ProjectGraphNode project, OutputLine line)
=> throw new InvalidOperationException();

public void ReportProcessOutput(OutputLine line)
=> throw new InvalidOperationException();
=> underlyingReporter.ReportProcessOutput(line);

public void Report(MessageDescriptor descriptor, string prefix, object?[] args)
=> underlyingReporter.Report(descriptor, _prefix + prefix, args);
Expand Down
17 changes: 7 additions & 10 deletions src/BuiltInTools/dotnet-watch/Internal/ConsoleReporter.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Build.Graph;

namespace Microsoft.DotNet.Watch
{
/// <summary>
Expand All @@ -15,16 +13,15 @@ internal sealed class ConsoleReporter(IConsole console, bool verbose, bool quiet
public bool IsQuiet { get; } = quiet;
public bool SuppressEmojis { get; } = suppressEmojis;

private readonly object _writeLock = new();

public bool EnableProcessOutputReporting
=> false;
private readonly Lock _writeLock = new();

public void ReportProcessOutput(OutputLine line)
=> throw new InvalidOperationException();

public void ReportProcessOutput(ProjectGraphNode project, OutputLine line)
=> throw new InvalidOperationException();
{
lock (_writeLock)
{
(line.IsError ? console.Error : console.Out).WriteLine(line.Content);
}
}

private void WriteLine(TextWriter writer, string message, ConsoleColor? color, string emoji)
{
Expand Down
12 changes: 9 additions & 3 deletions src/BuiltInTools/dotnet-watch/Internal/IReporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,19 @@ public bool IsVerbose
=> false;

/// <summary>
/// True to call <see cref="ReportProcessOutput"/> when launched process writes to standard output.
/// If true, the output of the process will be prefixed with the project display name.
/// Used for testing.
/// </summary>
bool EnableProcessOutputReporting { get; }
public bool PrefixProcessOutput
=> false;

/// <summary>
/// Reports the output of a process that is being watched.
/// </summary>
/// <remarks>
/// Not used to report output of dotnet-build processed launched by dotnet-watch to build or evaluate projects.
/// </remarks>
void ReportProcessOutput(OutputLine line);
void ReportProcessOutput(ProjectGraphNode project, OutputLine line);

void Report(MessageDescriptor descriptor, params object?[] args)
=> Report(descriptor, prefix: "", args);
Expand Down
17 changes: 5 additions & 12 deletions src/BuiltInTools/dotnet-watch/Internal/NullReporter.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Build.Graph;

namespace Microsoft.DotNet.Watch
{
/// <summary>
Expand All @@ -11,20 +9,15 @@ namespace Microsoft.DotNet.Watch
/// </summary>
internal sealed class NullReporter : IReporter
{
private NullReporter()
{ }

public static IReporter Singleton { get; } = new NullReporter();

public bool EnableProcessOutputReporting
=> false;
private NullReporter()
{
}

public void ReportProcessOutput(OutputLine line)
=> throw new InvalidOperationException();

public void ReportProcessOutput(ProjectGraphNode project, OutputLine line)
=> throw new InvalidOperationException();

{
}

public void Report(MessageDescriptor descriptor, string prefix, object?[] args)
{
Expand Down
11 changes: 5 additions & 6 deletions src/BuiltInTools/dotnet-watch/Internal/ProcessRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,10 @@ public static async Task<int> RunAsync(ProcessSpec processSpec, IReporter report

var onOutput = processSpec.OnOutput;

// allow tests to watch for application output:
if (reporter.EnableProcessOutputReporting)
{
onOutput += line => reporter.ReportProcessOutput(line);
}
// If output isn't already redirected (build invocation) we redirect it to the reporter.
// The reporter synchronizes the output of the process with the reporter output,
// so that the printed lines don't interleave.
onOutput ??= line => reporter.ReportProcessOutput(line);

using var process = CreateProcess(processSpec, onOutput, state, reporter);

Expand Down Expand Up @@ -186,7 +185,7 @@ private static Process CreateProcess(ProcessSpec processSpec, Action<OutputLine>
FileName = processSpec.Executable,
UseShellExecute = false,
WorkingDirectory = processSpec.WorkingDirectory,
RedirectStandardOutput = onOutput != null,
RedirectStandardOutput = onOutput != null,
RedirectStandardError = onOutput != null,
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,9 @@ internal sealed class ProjectSpecificReporter(ProjectGraphNode node, IReporter u
public bool IsVerbose
=> underlyingReporter.IsVerbose;

public bool EnableProcessOutputReporting
=> underlyingReporter.EnableProcessOutputReporting;

public void ReportProcessOutput(ProjectGraphNode project, OutputLine line)
=> underlyingReporter.ReportProcessOutput(project, line);

public void ReportProcessOutput(OutputLine line)
=> ReportProcessOutput(node, line);
=> underlyingReporter.ReportProcessOutput(
underlyingReporter.PrefixProcessOutput ? line with { Content = $"[{_projectDisplayName}] {line.Content}" } : line);

public void Report(MessageDescriptor descriptor, string prefix, object?[] args)
=> underlyingReporter.Report(descriptor, $"[{_projectDisplayName}] {prefix}", args);
Expand Down
31 changes: 17 additions & 14 deletions test/dotnet-watch.Tests/HotReload/RuntimeProcessLauncherTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

#nullable enable

using System.Collections.Immutable;
using System.Runtime.CompilerServices;

namespace Microsoft.DotNet.Watch.UnitTests;
Expand Down Expand Up @@ -139,6 +138,8 @@ public async Task UpdateAndRudeEdit(TriggerEvent trigger)
{
var testAsset = CopyTestAsset("WatchAppMultiProc", trigger);

var tfm = ToolsetInfo.CurrentTargetFramework;

var workingDirectory = testAsset.Path;
var hostDir = Path.Combine(testAsset.Path, "Host");
var hostProject = Path.Combine(hostDir, "Host.csproj");
Expand Down Expand Up @@ -216,18 +217,18 @@ async Task MakeValidDependencyChange()
{
var hasUpdateSourceA = w.CreateCompletionSource();
var hasUpdateSourceB = w.CreateCompletionSource();
w.Reporter.OnProjectProcessOutput += (projectPath, line) =>
w.Reporter.OnProcessOutput += line =>
{
if (line.Content.Contains("<Updated Lib>"))
{
if (projectPath == serviceProjectA)
if (line.Content.StartsWith($"[A ({tfm})]"))
{
if (!hasUpdateSourceA.Task.IsCompleted)
{
hasUpdateSourceA.SetResult();
}
}
else if (projectPath == serviceProjectB)
else if (line.Content.StartsWith($"[B ({tfm})]"))
{
if (!hasUpdateSourceB.Task.IsCompleted)
{
Expand All @@ -236,7 +237,7 @@ async Task MakeValidDependencyChange()
}
else
{
Assert.Fail("Only service projects should be updated");
Assert.Fail($"Only service projects should be updated: '{line.Content}'");
}
}
};
Expand Down Expand Up @@ -270,9 +271,9 @@ public static void Common()
async Task MakeRudeEditChange()
{
var hasUpdateSource = w.CreateCompletionSource();
w.Reporter.OnProjectProcessOutput += (projectPath, line) =>
w.Reporter.OnProcessOutput += line =>
{
if (projectPath == serviceProjectA && line.Content.Contains("Started A: 2"))
if (line.Content.StartsWith($"[A ({tfm})]") && line.Content.Contains("Started A: 2"))
{
hasUpdateSource.SetResult();
}
Expand All @@ -297,6 +298,7 @@ async Task MakeRudeEditChange()
public async Task UpdateAppliedToNewProcesses(bool sharedOutput)
{
var testAsset = CopyTestAsset("WatchAppMultiProc", sharedOutput);
var tfm = ToolsetInfo.CurrentTargetFramework;

if (sharedOutput)
{
Expand All @@ -322,21 +324,21 @@ public async Task UpdateAppliedToNewProcesses(bool sharedOutput)

var hasUpdateA = new SemaphoreSlim(initialCount: 0);
var hasUpdateB = new SemaphoreSlim(initialCount: 0);
w.Reporter.OnProjectProcessOutput += (projectPath, line) =>
w.Reporter.OnProcessOutput += line =>
{
if (line.Content.Contains("<Updated Lib>"))
{
if (projectPath == serviceProjectA)
if (line.Content.StartsWith($"[A ({tfm})]"))
{
hasUpdateA.Release();
}
else if (projectPath == serviceProjectB)
else if (line.Content.StartsWith($"[B ({tfm})]"))
{
hasUpdateB.Release();
}
else
{
Assert.Fail("Only service projects should be updated");
Assert.Fail($"Only service projects should be updated: '{line.Content}'");
}
}
};
Expand Down Expand Up @@ -395,6 +397,7 @@ public enum UpdateLocation
public async Task HostRestart(UpdateLocation updateLocation)
{
var testAsset = CopyTestAsset("WatchAppMultiProc", updateLocation);
var tfm = ToolsetInfo.CurrentTargetFramework;

var workingDirectory = testAsset.Path;
var hostDir = Path.Combine(testAsset.Path, "Host");
Expand All @@ -411,17 +414,17 @@ public async Task HostRestart(UpdateLocation updateLocation)
var restartRequested = w.Reporter.RegisterSemaphore(MessageDescriptor.RestartRequested);

var hasUpdate = new SemaphoreSlim(initialCount: 0);
w.Reporter.OnProjectProcessOutput += (projectPath, line) =>
w.Reporter.OnProcessOutput += line =>
{
if (line.Content.Contains("<Updated>"))
{
if (projectPath == hostProject)
if (line.Content.StartsWith($"[Host ({tfm})]"))
{
hasUpdate.Release();
}
else
{
Assert.Fail("Only service projects should be updated");
Assert.Fail($"Only service projects should be updated: '{line.Content}'");
}
}
};
Expand Down
16 changes: 5 additions & 11 deletions test/dotnet-watch.Tests/MsBuildFileSetFactoryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public class MsBuildFileSetFactoryTest(ITestOutputHelper output)
private readonly TestReporter _reporter = new(output);
private readonly TestAssetsManager _testAssets = new(output);

private string MuxerPath
private static string MuxerPath
=> TestContext.Current.ToolsetUnderTest.DotNetHostPath;

private static string InspectPath(string path, string rootDir)
Expand Down Expand Up @@ -329,9 +329,6 @@ public async Task ProjectReferences_Graph()
MuxerPath: MuxerPath,
WorkingDirectory: testDirectory);

var output = new List<string>();
_reporter.OnProcessOutput += line => output.Add(line.Content);

var filesetFactory = new MSBuildFileSetFactory(projectA, buildArguments: ["/p:_DotNetWatchTraceOutput=true"], options, _reporter);

var result = await filesetFactory.TryCreateAsync(requireProjectGraph: null, CancellationToken.None);
Expand Down Expand Up @@ -367,7 +364,7 @@ public async Task ProjectReferences_Graph()
"Collecting watch items from 'F'",
"Collecting watch items from 'G'",
],
output.Where(l => l.Contains("Collecting watch items from")).Select(l => l.Trim()).Order());
_reporter.Messages.Where(l => l.text.Contains("Collecting watch items from")).Select(l => l.text.Trim()).Order());
}

[Fact]
Expand All @@ -390,17 +387,14 @@ public async Task MsbuildOutput()
MuxerPath: MuxerPath,
WorkingDirectory: Path.GetDirectoryName(project1Path)!);

var output = new List<string>();
_reporter.OnProcessOutput += line => output.Add($"{(line.IsError ? "[stderr]" : "[stdout]")} {line.Content}");

var factory = new MSBuildFileSetFactory(project1Path, buildArguments: [], options, _reporter);
var result = await factory.TryCreateAsync(requireProjectGraph: null, CancellationToken.None);
Assert.Null(result);

// note: msbuild prints errors to stdout:
// note: msbuild prints errors to stdout, we match the pattern and report as error:
AssertEx.Equal(
$"[stdout] {project1Path} : error NU1201: Project Project2 is not compatible with net462 (.NETFramework,Version=v4.6.2). Project Project2 supports: netstandard2.1 (.NETStandard,Version=v2.1)",
output.Single(l => l.Contains("error NU1201")));
(MessageSeverity.Error, $"{project1Path} : error NU1201: Project Project2 is not compatible with net462 (.NETFramework,Version=v4.6.2). Project Project2 supports: netstandard2.1 (.NETStandard,Version=v2.1)"),
_reporter.Messages.Single(l => l.text.Contains("error NU1201")));
}

private Task<EvaluationResult> Evaluate(TestAsset projectPath)
Expand Down
10 changes: 2 additions & 8 deletions test/dotnet-watch.Tests/Utilities/MockReporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,15 @@

#nullable enable

using Microsoft.Build.Graph;

namespace Microsoft.DotNet.Watch.UnitTests;

internal class MockReporter : IReporter
{
public readonly List<string> Messages = [];

public bool EnableProcessOutputReporting => false;

public void ReportProcessOutput(OutputLine line)
=> throw new InvalidOperationException();

public void ReportProcessOutput(ProjectGraphNode project, OutputLine line)
=> throw new InvalidOperationException();
{
}

public void Report(MessageDescriptor descriptor, string prefix, object?[] args)
{
Expand Down
Loading
Loading