Fix ServiceController test (#57116)

src/libraries/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceController.cs

@@ -283,7 +283,7 @@ public string ServiceName
         }
 
         /// <summary>
-        /// A set of services on which the given service object is depend upon.
+        /// A set of services on which the given service object is dependent upon.
         /// </summary>
         public unsafe ServiceController[] ServicesDependedOn
         {

src/libraries/System.ServiceProcess.ServiceController/tests/ServiceBaseTests.cs

@@ -183,7 +183,7 @@ public void TestOnContinueBeforePause()
         public void LogWritten()
         {
             string serviceName = Guid.NewGuid().ToString();
-            // If the username is null, then the service is created under LocalSystem Account which have access to EventLog.
+            // If the username is null, then the service is created under LocalSystem Account which has access to EventLog.
             var testService = new TestServiceProvider(serviceName);
             Assert.True(EventLog.SourceExists(serviceName));
             testService.DeleteTestServices();

src/libraries/System.ServiceProcess.ServiceController/tests/ServiceControllerTests.cs

@@ -148,18 +148,16 @@ public void GetServices_FindSelf()
         [ConditionalFact(nameof(IsProcessElevated))]
         public void Dependencies()
         {
-            // The test service creates a number of dependent services, each of which is depended on
-            // by all the services created after it.
             var controller = new ServiceController(_testService.TestServiceName);
             Assert.Equal(0, controller.DependentServices.Length);
             Assert.Equal(1, controller.ServicesDependedOn.Length);
 
-            var dependentController = new ServiceController(_testService.TestServiceName + ".Dependent");
-            Assert.Equal(1, dependentController.DependentServices.Length);
-            Assert.Equal(0, dependentController.ServicesDependedOn.Length);
+            var prerequisiteServiceController = new ServiceController(_testService.TestServiceName + ".Prerequisite");
+            Assert.Equal(1, prerequisiteServiceController.DependentServices.Length);
+            Assert.Equal(0, prerequisiteServiceController.ServicesDependedOn.Length);
 
-            Assert.Equal(controller.ServicesDependedOn[0].ServiceName, dependentController.ServiceName);
-            Assert.Equal(dependentController.DependentServices[0].ServiceName, controller.ServiceName);
+            Assert.Equal(controller.ServicesDependedOn[0].ServiceName, prerequisiteServiceController.ServiceName);
+            Assert.Equal(prerequisiteServiceController.DependentServices[0].ServiceName, controller.ServiceName);
         }
 
         [ConditionalFact(nameof(IsProcessElevated))]
@@ -168,7 +166,7 @@ public void ServicesStartMode()
             var controller = new ServiceController(_testService.TestServiceName);
             Assert.Equal(ServiceStartMode.Manual, controller.StartType);
 
-            // Check for the startType of the dependent services.
+            // Check for the startType of the services that depend on the test service
             for (int i = 0; i < controller.DependentServices.Length; i++)
             {
                 Assert.Equal(ServiceStartMode.Disabled, controller.DependentServices[i].StartType);

src/libraries/System.ServiceProcess.ServiceController/tests/ServiceControllerTests.netcoreapp.cs

@@ -12,8 +12,15 @@ public partial class ServiceControllerTests : IDisposable
         public void Stop_FalseArg_WithDependentServices_ThrowsInvalidOperationException()
         {
             var controller = new ServiceController(_testService.TestServiceName);
-            controller.WaitForStatus(ServiceControllerStatus.Running, _testService.ControlTimeout);
-            Assert.Throws<InvalidOperationException>(() => controller.Stop(stopDependentServices: false));
+            Assert.Equal(0, controller.DependentServices.Length);
+            Assert.Equal(1, controller.ServicesDependedOn.Length);
+
+            var prerequisiteServiceController = new ServiceController(_testService.TestServiceName + ".Prerequisite");
+            Assert.Equal(1, prerequisiteServiceController.DependentServices.Length);
+            Assert.Equal(0, prerequisiteServiceController.ServicesDependedOn.Length);
+
+            prerequisiteServiceController.WaitForStatus(ServiceControllerStatus.Running, _testService.ControlTimeout);
+            Assert.Throws<InvalidOperationException>(() => prerequisiteServiceController.Stop(stopDependentServices: false));
         }
 
         [ConditionalFact(nameof(IsProcessElevated))]
@@ -35,6 +42,7 @@ public void StopTheServiceAndItsDependentsManually()
             var controller = new ServiceController(_testService.TestServiceName);
             controller.WaitForStatus(ServiceControllerStatus.Running, _testService.ControlTimeout);
 
+            // stop the services that depend on this service
             foreach (var dependentService in controller.DependentServices)
             {
                 dependentService.Stop(stopDependentServices: false);

src/libraries/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestService.cs

@@ -12,7 +12,7 @@ public class TestService : ServiceBase
         // To view tracing, use DbgView from sysinternals.com;
         // run it elevated, check "Capture>Global Win32" and "Capture>Win32",
         // and filter to just messages beginning with "##"
-        internal const bool DebugTracing = false;
+        internal const bool DebugTracing = false; // toggle in TestServiceProvider.cs as well
 
         private bool _disposed;
         private Task _waitClientConnect;

src/libraries/System.ServiceProcess.ServiceController/tests/System.ServiceProcess.ServiceController.TestService/TestServiceInstaller.cs

@@ -31,6 +31,7 @@ public TestServiceInstaller()
 
         public string ServiceCommandLine { get; set; }
 
+        // Install and start the test service, after starting any prerequisite services it depends on
         public unsafe void Install()
         {
             string username = Username;
@@ -53,6 +54,7 @@ public unsafe void Install()
             }
 
             // Build servicesDependedOn string
+            // These are prerequisite services that must be started before this service
             string servicesDependedOn = null;
             if (ServicesDependedOn.Length > 0)
             {
@@ -75,6 +77,8 @@ public unsafe void Install()
                 if (serviceManagerHandle.IsInvalid)
                     throw new InvalidOperationException("Cannot open Service Control Manager");
 
+                TestService.DebugTrace($"TestServiceInstaller: creating service {ServiceName} with prerequisite services {servicesDependedOn}");
+
                 // Install the service
                 using (var serviceHandle = new SafeServiceHandle(Interop.Advapi32.CreateService(serviceManagerHandle, ServiceName,
                     DisplayName, Interop.Advapi32.ServiceAccessOptions.ACCESS_TYPE_ALL, Interop.Advapi32.ServiceTypeOptions.SERVICE_TYPE_WIN32_OWN_PROCESS,
@@ -102,11 +106,15 @@ public unsafe void Install()
                     {
                         if (svc.Status != ServiceControllerStatus.Running)
                         {
-                            TestService.DebugTrace("TestServiceInstaller: instructing ServiceController to Start service " + ServiceName);
+                            TestService.DebugTrace($"TestServiceInstaller: instructing ServiceController to start service {ServiceName}");
                             svc.Start();
                             if (!ServiceName.StartsWith("PropagateExceptionFromOnStart"))
                                 svc.WaitForStatus(ServiceControllerStatus.Running, TimeSpan.FromSeconds(120));
                         }
+                        else
+                        {
+                            TestService.DebugTrace("TestServiceInstaller: service {ServiceName} already running");
+                        }
                     }
                 }
             }

src/libraries/System.ServiceProcess.ServiceController/tests/TestServiceProvider.cs

@@ -13,7 +13,7 @@ internal sealed class TestServiceProvider
         // To view tracing, use DbgView from sysinternals.com;
         // run it elevated, check "Capture>Global Win32" and "Capture>Win32",
         // and filter to just messages beginning with "##"
-        internal const bool DebugTracing = false;
+        internal const bool DebugTracing = false; // toggle in TestService.cs as well
 
         private const int readTimeout = 60000;
 
@@ -56,27 +56,30 @@ public NamedPipeClientStream Client
         public readonly string TestServiceName;
         public readonly string TestServiceDisplayName;
 
-        private readonly TestServiceProvider _dependentServices;
+        private readonly TestServiceProvider _prerequisiteServices;
+
+        // Creates a test service with a prerequisite service
         public TestServiceProvider()
         {
             TestMachineName = ".";
             ControlTimeout = TimeSpan.FromSeconds(120);
             TestServiceName = Guid.NewGuid().ToString();
             TestServiceDisplayName = "Test Service " + TestServiceName;
 
-            _dependentServices = new TestServiceProvider(TestServiceName + ".Dependent");
+            _prerequisiteServices = new TestServiceProvider(TestServiceName + ".Prerequisite");
 
             // Create the service
             CreateTestServices();
         }
 
+        // Creates a test service with no prerequisite services
         public TestServiceProvider(string serviceName)
         {
             TestMachineName = ".";
             ControlTimeout = TimeSpan.FromSeconds(120);
             TestServiceName = serviceName;
             TestServiceDisplayName = "Test Service " + TestServiceName;
-            
+
             // Create the service
             CreateTestServices();
         }
@@ -94,16 +97,18 @@ public async Task<byte> ReadPipeAsync()
 
         private void CreateTestServices()
         {
+            DebugTrace($"TestServiceProvider: Creating test service {TestServiceName}");
             TestServiceInstaller testServiceInstaller = new TestServiceInstaller();
 
             testServiceInstaller.ServiceName = TestServiceName;
             testServiceInstaller.DisplayName = TestServiceDisplayName;
             testServiceInstaller.Description = "__Dummy Test Service__";
             testServiceInstaller.Username = null;
 
-            if (_dependentServices != null)
+            if (_prerequisiteServices != null)
             {
-                testServiceInstaller.ServicesDependedOn = new string[] { _dependentServices.TestServiceName };
+                DebugTrace($"TestServiceProvider: .. with prequisite services {_prerequisiteServices.TestServiceName}");
+                testServiceInstaller.ServicesDependedOn = new string[] { _prerequisiteServices.TestServiceName };
             }
 
             string processName = Process.GetCurrentProcess().MainModule.FileName;
@@ -122,8 +127,9 @@ private void CreateTestServices()
             }
 
             testServiceInstaller.ServiceCommandLine = $"\"{processName}\" {arguments}";
-
+            DebugTrace($"TestServiceProvider: Executing {testServiceInstaller.ServiceCommandLine}");
             testServiceInstaller.Install();
+            DebugTrace("TestServiceProvider: Completed install of test service");
         }
 
         public void DeleteTestServices()
@@ -140,14 +146,16 @@ public void DeleteTestServices()
                 TestServiceInstaller testServiceInstaller = new TestServiceInstaller();
                 testServiceInstaller.ServiceName = TestServiceName;
                 testServiceInstaller.RemoveService();
+                DebugTrace("TestServiceProvider: Removed test service");
             }
             finally
             {
-                // Lets be sure to try and clean up dependenct services even if something goes
+                // Lets be sure to try and clean up prerequisite services even if something goes
                 // wrong with the full removal of the other service.
-                if (_dependentServices != null)
+                if (_prerequisiteServices != null)
                 {
-                    _dependentServices.DeleteTestServices();
+                    _prerequisiteServices.DeleteTestServices();
+                    DebugTrace("TestServiceProvider: Deleted test services");
                 }
             }
         }