Fix flaky test failure in ServerCommandTest

Author: ajaybhargavb

src/Microsoft.AspNetCore.Razor.Tools/ServerCommand.cs

@@ -25,7 +25,10 @@ public ServerCommand(Application parent)
         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 @@ protected virtual void ExecuteServerCore(ConnectionHost host, CompilerHost compi
             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 @@ internal FileStream WritePidFile()
             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 @@ internal FileStream WritePidFile()
 
             return fileStream;
         }
+
+        // Internal for testing.
+        internal virtual string GetPidFilePath(Func<string, string> 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;
+        }
     }
 }

test/Microsoft.AspNetCore.Razor.Tools.Test/Infrastructure/ServerUtilities.cs

@@ -116,7 +116,6 @@ private class TestableServerCommand : ServerCommand
             private readonly CancellationToken _cancellationToken;
             private readonly TimeSpan? _keepAlive;
 
-
             public TestableServerCommand(
                 ConnectionHost host,
                 CompilerHost compilerHost,
@@ -146,6 +145,12 @@ protected override void ExecuteServerCore(
                     _eventBus ?? eventBus,
                     _keepAlive ?? keepAlive);
             }
+
+            protected override FileStream WritePidFile()
+            {
+                // Disable writing PID file as it is tested separately.
+                return null;
+            }
         }
     }
 }

test/Microsoft.AspNetCore.Razor.Tools.Test/ServerCommandTest.cs

@@ -13,23 +13,23 @@ 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);
 
             // Act & Assert
             try
             {
-                using (var _ = server.WritePidFile())
+                using (var _ = server.WritePidFile(directoryPath))
                 {
                     Assert.True(File.Exists(path));
 
@@ -47,15 +47,64 @@ public void WritePidFile_WorksAsExpected()
             }
             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,