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

dotnet watch run shouldn't disable the browser refresh logic inject… #15340

Merged
merged 1 commit into from
Jan 14, 2021
Merged
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
43 changes: 21 additions & 22 deletions src/BuiltInTools/dotnet-watch/LaunchBrowserFilter.cs
Original file line number Diff line number Diff line change
@@ -25,9 +25,8 @@ public sealed class LaunchBrowserFilter : IWatchFilter, IAsyncDisposable
private readonly bool _suppressBrowserRefresh;
private readonly string _browserPath;

private bool _canLaunchBrowser;
private bool _attemptedBrowserLaunch;
private Process _browserProcess;
private bool _browserLaunched;
private BrowserRefreshServer _refreshServer;
private IReporter _reporter;
private string _launchPath;
@@ -59,7 +58,6 @@ public async ValueTask ProcessAsync(DotNetWatchContext context, CancellationToke
if (CanLaunchBrowser(context, out var launchPath))
{
context.Reporter.Verbose("dotnet-watch is configured to launch a browser on ASP.NET Core application startup.");
_canLaunchBrowser = true;
_launchPath = launchPath;
_cancellationToken = cancellationToken;

@@ -81,14 +79,10 @@ public async ValueTask ProcessAsync(DotNetWatchContext context, CancellationToke
}
}
}

if (_canLaunchBrowser)
else if (!_suppressBrowserRefresh)
{
if (context.Iteration > 0)
{
// We've detected a change. Notify the browser.
await SendMessage(WaitMessage, cancellationToken);
}
// We've detected a change. Notify the browser.
await SendMessage(WaitMessage, cancellationToken);
}
}

@@ -117,18 +111,30 @@ private void OnOutput(object sender, DataReceivedEventArgs eventArgs)
var process = (Process)sender;
process.OutputDataReceived -= OnOutput;

if (!_browserLaunched)
if (!_attemptedBrowserLaunch)
{
_attemptedBrowserLaunch = true;

_reporter.Verbose("Launching browser.");

try
{
LaunchBrowser(launchUrl);
_browserLaunched = true;
}
catch (Exception ex)
{
_reporter.Output($"Unable to launch browser: {ex}");
_canLaunchBrowser = false;
_reporter.Verbose($"An exception occurred when attempting to launch a browser: {ex}");
_browserProcess = null;
}

if (_browserProcess is null || _browserProcess.HasExited)
{
// dotnet-watch, by default, relies on URL file association to launch browsers. On Windows and MacOS, this works fairly well
// where URLs are associated with the default browser. On Linux, this is a bit murky.
// From emperical observation, it's noted that failing to launch a browser results in either Process.Start returning a null-value
// or for the process to have immediately exited.
// We can use this to provide a helpful message.
_reporter.Output($"Unable to launch the browser. Navigate to {launchUrl}");
}
}
else
@@ -175,13 +181,6 @@ private static bool CanLaunchBrowser(DotNetWatchContext context, out string laun
return false;
}

if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && !RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
// Launching a browser requires file associations that are not available in all operating systems.
reporter.Verbose("Browser refresh is only supported in Windows and MacOS.");
return false;
}

var dotnetCommand = context.ProcessSpec.Arguments.FirstOrDefault();
if (!string.Equals(dotnetCommand, "run", StringComparison.Ordinal))
{
@@ -236,4 +235,4 @@ public async ValueTask DisposeAsync()
}
}
}
}
}
7 changes: 4 additions & 3 deletions src/Tests/dotnet-watch.Tests/BrowserLaunchTests.cs
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
using System.IO;
using System.Threading.Tasks;
using Microsoft.NET.TestFramework;
using Xunit;
using Xunit.Abstractions;

namespace Microsoft.DotNet.Watcher.Tools
@@ -22,7 +23,7 @@ public BrowserLaunchTests(ITestOutputHelper logger)
_logger = logger;
}

[PlatformSpecificFact(Xunit.TestPlatforms.Windows | Xunit.TestPlatforms.OSX)]
[Fact]
public async Task LaunchesBrowserOnStart()
{
var expected = "watch : Launching browser: https://localhost:5001/";
@@ -40,7 +41,7 @@ public async Task LaunchesBrowserOnStart()
await app.Process.GetOutputLineStartsWithAsync(expected, TimeSpan.FromMinutes(2));
}

[PlatformSpecificFact(Xunit.TestPlatforms.Windows | Xunit.TestPlatforms.OSX)]
[Fact]
public async Task RefreshesBrowserOnChange()
{
var launchBrowserMessage = "watch : Launching browser: https://localhost:5001/";
@@ -64,7 +65,7 @@ public async Task RefreshesBrowserOnChange()
await app.Process.GetOutputLineStartsWithAsync(refreshBrowserMessage, TimeSpan.FromMinutes(2));
}

[PlatformSpecificFact(Xunit.TestPlatforms.Windows | Xunit.TestPlatforms.OSX)]
[Fact]
public async Task UsesBrowserSpecifiedInEnvironment()
{
var launchBrowserMessage = "watch : Launching browser: mycustombrowser.bat https://localhost:5001/";