Fix a flaky compiler suite

The intent of ServerRespectsAppConfig was to verify the server actually read the default timeout value from the application configuration file.  The test validated this in a very indirect way:

- Changing the value in app.config
- Starting the server
- Verifying it stopped in the given time frame

This can fail for a number of reasons: computer load during the test, another process communicating with the server, etc ...

Changed the test to just directly test the value being read from application settings.

Justification for ask mode: this test was flaky enough to break signed builds.

src/Compilers/Server/PortableServer/PortableBuildServerController.cs

@@ -41,7 +41,7 @@ protected override IClientConnectionHost CreateClientConnectionHost(string pipeN
             return connectionHost;
         }
 
-        protected override TimeSpan? GetKeepAliveTimeout()
+        protected internal override TimeSpan? GetKeepAliveTimeout()
         {
             return null;
         }

src/Compilers/Server/ServerShared/BuildServerController.cs

@@ -37,7 +37,7 @@ internal int Run(string[] args)
                 : RunServer(pipeName, cancellationToken: cancellationTokenSource.Token);
         }
 
-        protected abstract TimeSpan? GetKeepAliveTimeout();
+        protected internal abstract TimeSpan? GetKeepAliveTimeout();
 
         protected abstract string GetDefaultPipeName();
 

src/Compilers/Server/VBCSCompiler/DesktopBuildServerController.cs

@@ -14,11 +14,21 @@
 using System.Globalization;
 using Microsoft.CodeAnalysis.CommandLine;
 using System.Runtime.InteropServices;
+using System.Collections.Specialized;
 
 namespace Microsoft.CodeAnalysis.CompilerServer
 {
     internal class DesktopBuildServerController : BuildServerController
     {
+        internal const string KeepAliveSettingName = "keepalive";
+
+        private readonly NameValueCollection _appSettings;
+
+        internal DesktopBuildServerController(NameValueCollection appSettings = null)
+        {
+            _appSettings = appSettings ?? ConfigurationManager.AppSettings;
+        }
+
         protected override IClientConnectionHost CreateClientConnectionHost(string pipeName)
         {
             // VBCSCompiler is installed in the same directory as csc.exe and vbc.exe which is also the 
@@ -29,12 +39,12 @@ protected override IClientConnectionHost CreateClientConnectionHost(string pipeN
             return new NamedPipeClientConnectionHost(compilerServerHost, pipeName);
         }
 
-        protected override TimeSpan? GetKeepAliveTimeout()
+        protected internal override TimeSpan? GetKeepAliveTimeout()
         {
             try
             {
                 int keepAliveValue;
-                string keepAliveStr = ConfigurationManager.AppSettings["keepalive"];
+                string keepAliveStr = _appSettings[KeepAliveSettingName];
                 if (int.TryParse(keepAliveStr, NumberStyles.Integer, CultureInfo.InvariantCulture, out keepAliveValue) &&
                     keepAliveValue >= 0)
                 {

src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs

@@ -1300,31 +1300,6 @@ public async Task Utf8OutputInRspFileVbc()
             }
         }
 
-        [Fact, WorkItem(1095079, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1095079")]
-        public async Task ServerRespectsAppConfig()
-        {
-            var exeConfigPath = Path.Combine(CompilerDirectory, CompilerServerExeName + ".config");
-            var doc = new XmlDocument();
-            using (XmlReader reader = XmlReader.Create(exeConfigPath, new XmlReaderSettings { DtdProcessing = DtdProcessing.Prohibit, XmlResolver = null }))
-            {
-                doc.Load(reader);
-            }
-            var root = doc.DocumentElement;
-
-            root.SelectSingleNode("appSettings/add/@value").Value = "1";
-            doc.Save(exeConfigPath);
-
-            var proc = StartProcess(CompilerServerExecutable, "");
-            await Task.Delay(TimeSpan.FromSeconds(3)).ConfigureAwait(false); // Give 2s leeway
-
-            var exited = proc.HasExited;
-            if (!exited)
-            {
-                Kill(proc);
-                Assert.True(false, "Compiler server did not exit in time");
-            }
-        }
-
         [Fact]
         public void BadKeepAlive1()
         {

src/Compilers/Server/VBCSCompilerTests/DesktopBuildServerControllerTests.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Collections.Specialized;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.CompilerServer.UnitTests
+{
+    public class DesktopBuildServerControllerTests
+    {
+        public sealed class GetKeepAliveTimeoutTests
+        {
+            private readonly NameValueCollection _appSettings = new NameValueCollection();
+            private readonly DesktopBuildServerController _controller;
+
+            public GetKeepAliveTimeoutTests()
+            {
+                _controller = new DesktopBuildServerController(_appSettings);
+            }
+
+            [Fact]
+            public void Simple()
+            {
+                _appSettings[DesktopBuildServerController.KeepAliveSettingName] = "42";
+                Assert.Equal(TimeSpan.FromSeconds(42), _controller.GetKeepAliveTimeout());
+            }
+
+            [Fact]
+            public void InvalidNumber()
+            {
+                _appSettings[DesktopBuildServerController.KeepAliveSettingName] = "dog";
+                Assert.Equal(ServerDispatcher.DefaultServerKeepAlive, _controller.GetKeepAliveTimeout());
+            }
+
+            [Fact]
+            public void NoSetting()
+            {
+                Assert.Equal(ServerDispatcher.DefaultServerKeepAlive, _controller.GetKeepAliveTimeout());
+            }
+        }
+    }
+}

src/Compilers/Server/VBCSCompilerTests/VBCSCompilerTests.csproj

@@ -120,6 +120,7 @@
     <Compile Include="CompilerServerApiTest.cs" />
     <Compile Include="CompilerServerTests.cs" />
     <Compile Include="DesktopBuildClientTests.cs" />
+    <Compile Include="DesktopBuildServerControllerTests.cs" />
     <Compile Include="EndToEndDeterminismTest.cs" />
     <Compile Include="VBCSCompilerServerTests.cs" />
     <Compile Include="MockEngine.cs" />