From 7cba5ed59309b832aafe47f338399e3676b1afda Mon Sep 17 00:00:00 2001 From: Ajay Bhargav Baaskaran Date: Mon, 30 Apr 2018 13:02:15 -0700 Subject: [PATCH] Fix flaky test failure in ServerCommandTest --- .../ServerCommand.cs | 58 ++++++++++++----- .../Infrastructure/ServerUtilities.cs | 7 +- .../ServerCommandTest.cs | 65 ++++++++++++++++--- 3 files changed, 103 insertions(+), 27 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Tools/ServerCommand.cs b/src/Microsoft.AspNetCore.Razor.Tools/ServerCommand.cs index 44bb594b1c..a4aee01b3d 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/ServerCommand.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/ServerCommand.cs @@ -25,7 +25,10 @@ namespace Microsoft.AspNetCore.Razor.Tools internal ServerCommand(Application parent, string pipeName, int? keepAlive = null) : this(parent) { - Pipe.Values.Add(pipeName); + if (!string.IsNullOrEmpty(pipeName)) + { + Pipe.Values.Add(pipeName); + } if (keepAlive.HasValue) { @@ -119,8 +122,21 @@ namespace Microsoft.AspNetCore.Razor.Tools dispatcher.Run(); } - internal FileStream WritePidFile() + protected virtual FileStream WritePidFile() { + var path = GetPidFilePath(env => Environment.GetEnvironmentVariable(env)); + return WritePidFile(path); + } + + // Internal for testing. + internal virtual FileStream WritePidFile(string directoryPath) + { + if (string.IsNullOrEmpty(directoryPath)) + { + // Invalid path. Bail. + return null; + } + // To make all the running rzc servers more discoverable, We want to write the process Id and pipe name to a file. // The file contents will be in the following format, // @@ -133,24 +149,10 @@ namespace Microsoft.AspNetCore.Razor.Tools var processId = Process.GetCurrentProcess().Id; var fileName = $"rzc-{processId}"; - var path = Environment.GetEnvironmentVariable("DOTNET_BUILD_PIDFILE_DIRECTORY"); - if (string.IsNullOrEmpty(path)) - { - var homeEnvVariable = PlatformInformation.IsWindows ? "USERPROFILE" : "HOME"; - var homePath = Environment.GetEnvironmentVariable(homeEnvVariable); - if (string.IsNullOrEmpty(homePath)) - { - // Couldn't locate the user profile directory. Bail. - return null; - } - - path = Path.Combine(homePath, ".dotnet", "pids", "build"); - } - // Make sure the directory exists. - Directory.CreateDirectory(path); + Directory.CreateDirectory(directoryPath); - path = Path.Combine(path, fileName); + var path = Path.Combine(directoryPath, fileName); var fileStream = new FileStream(path, FileMode.Create, FileAccess.Write, FileShare.Read, DefaultBufferSize, FileOptions.DeleteOnClose); using (var writer = new StreamWriter(fileStream, Encoding.UTF8, DefaultBufferSize, leaveOpen: true)) @@ -162,5 +164,25 @@ namespace Microsoft.AspNetCore.Razor.Tools return fileStream; } + + // Internal for testing. + internal virtual string GetPidFilePath(Func getEnvironmentVariable) + { + var path = getEnvironmentVariable("DOTNET_BUILD_PIDFILE_DIRECTORY"); + if (string.IsNullOrEmpty(path)) + { + var homeEnvVariable = PlatformInformation.IsWindows ? "USERPROFILE" : "HOME"; + var homePath = getEnvironmentVariable(homeEnvVariable); + if (string.IsNullOrEmpty(homePath)) + { + // Couldn't locate the user profile directory. Bail. + return null; + } + + path = Path.Combine(homePath, ".dotnet", "pids", "build"); + } + + return path; + } } } diff --git a/test/Microsoft.AspNetCore.Razor.Tools.Test/Infrastructure/ServerUtilities.cs b/test/Microsoft.AspNetCore.Razor.Tools.Test/Infrastructure/ServerUtilities.cs index d6494d1d37..65e407f644 100644 --- a/test/Microsoft.AspNetCore.Razor.Tools.Test/Infrastructure/ServerUtilities.cs +++ b/test/Microsoft.AspNetCore.Razor.Tools.Test/Infrastructure/ServerUtilities.cs @@ -116,7 +116,6 @@ namespace Microsoft.AspNetCore.Razor.Tools private readonly CancellationToken _cancellationToken; private readonly TimeSpan? _keepAlive; - public TestableServerCommand( ConnectionHost host, CompilerHost compilerHost, @@ -146,6 +145,12 @@ namespace Microsoft.AspNetCore.Razor.Tools _eventBus ?? eventBus, _keepAlive ?? keepAlive); } + + protected override FileStream WritePidFile() + { + // Disable writing PID file as it is tested separately. + return null; + } } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerCommandTest.cs b/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerCommandTest.cs index cc77e3f240..d61fe7e09e 100644 --- a/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerCommandTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Tools.Test/ServerCommandTest.cs @@ -13,15 +13,15 @@ namespace Microsoft.AspNetCore.Razor.Tools { public class ServerCommandTest { - [Fact(Skip = "https://github.com/aspnet/Razor/issues/2310")] + [Fact] public void WritePidFile_WorksAsExpected() { // Arrange var expectedProcessId = Process.GetCurrentProcess().Id; var expectedRzcPath = typeof(ServerCommand).Assembly.Location; var expectedFileName = $"rzc-{expectedProcessId}"; - var homeEnvVariable = PlatformInformation.IsWindows ? "USERPROFILE" : "HOME"; - var path = Path.Combine(Environment.GetEnvironmentVariable(homeEnvVariable), ".dotnet", "pids", "build", expectedFileName); + var directoryPath = Path.Combine(Path.GetTempPath(), "RazorTest", Guid.NewGuid().ToString()); + var path = Path.Combine(directoryPath, expectedFileName); var pipeName = Guid.NewGuid().ToString(); var server = GetServerCommand(pipeName); @@ -29,7 +29,7 @@ namespace Microsoft.AspNetCore.Razor.Tools // Act & Assert try { - using (var _ = server.WritePidFile()) + using (var _ = server.WritePidFile(directoryPath)) { Assert.True(File.Exists(path)); @@ -47,15 +47,64 @@ namespace Microsoft.AspNetCore.Razor.Tools } finally { - // Delete the file in case the test fails. - if (File.Exists(path)) + // Cleanup after the test. + if (Directory.Exists(directoryPath)) { - File.Delete(path); + Directory.Delete(directoryPath, recursive: true); } } } - private ServerCommand GetServerCommand(string pipeName) + [Fact] + public void GetPidFilePath_ReturnsCorrectDefaultPath() + { + // Arrange + var expectedPath = Path.Combine("homeDir", ".dotnet", "pids", "build"); + var server = GetServerCommand(); + + // Act + var directoryPath = server.GetPidFilePath(getEnvironmentVariable: env => + { + if (env == "DOTNET_BUILD_PIDFILE_DIRECTORY") + { + return null; + } + + return "homeDir"; + }); + + // Assert + Assert.Equal(expectedPath, directoryPath); + } + + [Fact] + public void GetPidFilePath_UsesEnvironmentVariablePathIfSpecified() + { + // Arrange + var expectedPath = "/Some/directory/path/"; + var server = GetServerCommand(); + + // Act + var directoryPath = server.GetPidFilePath(getEnvironmentVariable: env => expectedPath); + + // Assert + Assert.Equal(expectedPath, directoryPath); + } + + [Fact] + public void GetPidFilePath_NullEnvironmentVariableValue_ReturnsNull() + { + // Arrange + var server = GetServerCommand(); + + // Act + var directoryPath = server.GetPidFilePath(getEnvironmentVariable: env => null); + + // Assert + Assert.Null(directoryPath); + } + + private ServerCommand GetServerCommand(string pipeName = null) { var application = new Application( CancellationToken.None,