From babc3489dfe4e87b539d50e9fbd37298c9856ce4 Mon Sep 17 00:00:00 2001
From: Steve Lee <slee@microsoft.com>
Date: Sun, 10 Sep 2017 14:37:07 -0700
Subject: [PATCH 1/6] [feature]

missed -ErrorAction Stop
---
 .../Microsoft.PowerShell.Management/Set-Service.Tests.ps1       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
index 5a30db2fe0b..01e2ea3d6e6 100644
--- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
+++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
@@ -42,7 +42,7 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows"
 
     It "Set-Service parameter validation for invalid values: <script>" -TestCases @(
         @{
-            script  = {Set-Service foo -StartupType bar};
+            script  = {Set-Service foo -StartupType bar -ErrorAction Stop};
             errorid = "CannotConvertArgumentNoMessage,Microsoft.PowerShell.Commands.SetServiceCommand"
         }
     ) {

From c67a75e775c1a3b5eddc7a9005f07cc1459d2a38 Mon Sep 17 00:00:00 2001
From: "Steve Lee [MSFT]" <slee@microsoft.com>
Date: Mon, 11 Sep 2017 14:57:11 -0700
Subject: [PATCH 2/6] [feature]

made cmdlet write error when using unsupported startuptype
re-enabled test
---
 .../commands/management/Service.cs                       | 9 +++++----
 .../resources/ServiceResources.resx                      | 3 +++
 .../Set-Service.Tests.ps1                                | 3 +--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
index c8986ab7089..c72aa2099c8 100644
--- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
+++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
@@ -2015,10 +2015,11 @@ protected override void BeginProcessing()
                         dwStartType = NativeMethods.SERVICE_DISABLED;
                         break;
                     default:
-                        Diagnostics.Assert(
-                            ((ServiceStartMode)(-1)) == StartupType,
-                            "bad StartupType");
-                        break;
+                        WriteNonTerminatingError(StartupType.ToString(), DisplayName, Name,
+                            new ArgumentException(), "CouldNotNewService",
+                            ServiceResources.UnsupportedStartupType,
+                            ErrorCategory.InvalidArgument);
+                        return;
                 }
 
                 // set up the double-null-terminated lpDependencies parameter
diff --git a/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx b/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx
index 0f99f066c8c..cf50d8453d0 100644
--- a/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx
+++ b/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx
@@ -201,4 +201,7 @@
   <data name="ComputerAccessDenied" xml:space="preserve">
     <value>The command cannot be used to configure the service '{0}' because access to computer '{1}' is denied. Run PowerShell as admin and run your command again.</value>
   </data>
+  <data name="UnsupportedStartupType" xml:space="preserve">
+    <value>The startup type '{0}' is not supported by New-Service.</value>
+  </data>
 </root>
diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
index 01e2ea3d6e6..aafdc4df7fa 100644
--- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
+++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
@@ -163,8 +163,7 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows"
             [System.Management.Automation.PSCredential]::new("username", 
             (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force)))
         },
-        # This test case fails due to #4803. Disabled for now.
-        # @{name = 'badstarttype'; parameter = "StartupType"; value = "System"},
+        @{name = 'badstarttype'; parameter = "StartupType"; value = "System"},
         @{name = 'winmgmt'     ; parameter = "DisplayName"; value = "foo"}
     ) {
         param($name, $parameter, $value)

From 4bde7fb37fcb9e50411a0ff40441203be6b6e653 Mon Sep 17 00:00:00 2001
From: "Steve Lee [MSFT]" <slee@microsoft.com>
Date: Mon, 11 Sep 2017 16:09:47 -0700
Subject: [PATCH 3/6] [feature] fix test so that a script block only runs if It
 is run

---
 .../Microsoft.PowerShell.Management/Set-Service.Tests.ps1  | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
index aafdc4df7fa..43dcb82a312 100644
--- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
+++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
@@ -22,15 +22,18 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows"
         @{parameter = "Status"      ; value = "Running"},
         @{parameter = "Status"      ; value = "Stopped"},
         @{parameter = "Status"      ; value = "Paused"},
-        @{parameter = "InputObject" ; value = (Get-Service | Select-Object -First 1)},
+        @{parameter = "InputObject" ; script = {Get-Service | Select-Object -First 1}},
         # cmdlet inherits this property, but it's not exposed as parameter so it should be $null
         @{parameter = "Include"     ; value = "foo", "bar" ; expectedNull = $true},
         # cmdlet inherits this property, but it's not exposed as parameter so it should be $null
         @{parameter = "Exclude"     ; value = "foo", "bar" ; expectedNull = $true}
     ) {
-        param($parameter, $value, $expectedNull)
+        param($parameter, $value, $script, $expectedNull)
 
         $setServiceCommand = [Microsoft.PowerShell.Commands.SetServiceCommand]::new()
+        if ($script -ne $Null) {
+            $value = & $script
+        }
         $setServiceCommand.$parameter = $value
         if ($expectedNull -eq $true) {
             $setServiceCommand.$parameter | Should BeNullOrEmpty

From 94561f48ace8159b0a121827f9ad961796f2d814 Mon Sep 17 00:00:00 2001
From: "Steve Lee [MSFT]" <slee@microsoft.com>
Date: Tue, 12 Sep 2017 14:44:22 -0700
Subject: [PATCH 4/6] [feature] turned redundant code into helper function used
 by New/Set-Service added tests for Set-Service and unsupported StartupTypes

---
 .../commands/management/Service.cs            | 83 +++++++++++--------
 .../resources/ServiceResources.resx           |  2 +-
 .../Set-Service.Tests.ps1                     | 28 ++++---
 3 files changed, 67 insertions(+), 46 deletions(-)

diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
index c72aa2099c8..e0c7118faf2 100644
--- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
+++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
@@ -1511,8 +1511,6 @@ public ServiceStartMode StartupType
         internal ServiceStartMode startupType = (ServiceStartMode)(-1);
 
 
-
-
         /// <summary>
         /// The following is the definition of the input parameter "Status".
         /// This specifies what state the service should be in (e.g. Running, Stopped,
@@ -1684,22 +1682,14 @@ protected override void ProcessRecord()
                             || (ServiceStartMode)(-1) != StartupType)
                         {
                             DWORD dwStartType = NativeMethods.SERVICE_NO_CHANGE;
-                            switch (StartupType)
+                            if ((ServiceStartMode)(-1) != StartupType &&
+                                !NativeMethods.GetNativeStartupType(StartupType, out dwStartType))
                             {
-                                case ServiceStartMode.Automatic:
-                                    dwStartType = NativeMethods.SERVICE_AUTO_START;
-                                    break;
-                                case ServiceStartMode.Manual:
-                                    dwStartType = NativeMethods.SERVICE_DEMAND_START;
-                                    break;
-                                case ServiceStartMode.Disabled:
-                                    dwStartType = NativeMethods.SERVICE_DISABLED;
-                                    break;
-                                default:
-                                    Diagnostics.Assert(
-                                        ((ServiceStartMode)(-1)) == StartupType,
-                                        "bad StartupType");
-                                    break;
+                                WriteNonTerminatingError(StartupType.ToString(), "Set-Service", Name,
+                                    new ArgumentException(), "CouldNotSetService",
+                                    ServiceResources.UnsupportedStartupType,
+                                    ErrorCategory.InvalidArgument);
+                                return;
                             }
                             bool succeeded = NativeMethods.ChangeServiceConfigW(
                                 hService,
@@ -2002,26 +1992,15 @@ protected override void BeginProcessing()
                         ErrorCategory.PermissionDenied);
                     return;
                 }
-                DWORD dwStartType = NativeMethods.SERVICE_AUTO_START;
-                switch (StartupType)
+                DWORD dwStartType = NativeMethods.SERVICE_NO_CHANGE;
+                if (! NativeMethods.GetNativeStartupType(StartupType, out dwStartType))
                 {
-                    case ServiceStartMode.Automatic:
-                        dwStartType = NativeMethods.SERVICE_AUTO_START;
-                        break;
-                    case ServiceStartMode.Manual:
-                        dwStartType = NativeMethods.SERVICE_DEMAND_START;
-                        break;
-                    case ServiceStartMode.Disabled:
-                        dwStartType = NativeMethods.SERVICE_DISABLED;
-                        break;
-                    default:
-                        WriteNonTerminatingError(StartupType.ToString(), DisplayName, Name,
-                            new ArgumentException(), "CouldNotNewService",
-                            ServiceResources.UnsupportedStartupType,
-                            ErrorCategory.InvalidArgument);
-                        return;
+                    WriteNonTerminatingError(StartupType.ToString(), "New-Service", Name,
+                        new ArgumentException(), "CouldNotNewService",
+                        ServiceResources.UnsupportedStartupType,
+                        ErrorCategory.InvalidArgument);
+                    return;
                 }
-
                 // set up the double-null-terminated lpDependencies parameter
                 IntPtr lpDependencies = IntPtr.Zero;
                 if (null != DependsOn)
@@ -2411,6 +2390,40 @@ [In] IntPtr lpPassword
         public static extern bool QueryInformationJobObject(SafeHandle hJob, int JobObjectInfoClass,
                                     ref JOBOBJECT_BASIC_PROCESS_ID_LIST lpJobObjectInfo,
                                     int cbJobObjectLength, IntPtr lpReturnLength);
+
+        /// <summary>
+        /// Get appropriate win32 StartupType
+        /// </summary>
+        /// <param name="StartupType">
+        /// StartupType provided by the user.
+        /// </param>
+        /// <param name="dwStartType">
+        /// Out parameter of the native win32 StartupType
+        /// </param>
+        /// <returns>
+        /// If a supported StartupType is provided, funciton returns true, otherwise false.
+        /// </returns>
+        internal static bool GetNativeStartupType(ServiceStartMode StartupType, out DWORD dwStartType)
+        {
+            bool success = true;
+            dwStartType = NativeMethods.SERVICE_NO_CHANGE;
+            switch (StartupType)
+            {
+                case ServiceStartMode.Automatic:
+                    dwStartType = NativeMethods.SERVICE_AUTO_START;
+                    break;
+                case ServiceStartMode.Manual:
+                    dwStartType = NativeMethods.SERVICE_DEMAND_START;
+                    break;
+                case ServiceStartMode.Disabled:
+                    dwStartType = NativeMethods.SERVICE_DISABLED;
+                    break;
+                default:
+                    success = false;
+                    break;
+            }
+            return success;
+        }
     }
     #endregion NativeMethods
 }
diff --git a/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx b/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx
index cf50d8453d0..60ecdd041f0 100644
--- a/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx
+++ b/src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx
@@ -202,6 +202,6 @@
     <value>The command cannot be used to configure the service '{0}' because access to computer '{1}' is denied. Run PowerShell as admin and run your command again.</value>
   </data>
   <data name="UnsupportedStartupType" xml:space="preserve">
-    <value>The startup type '{0}' is not supported by New-Service.</value>
+    <value>The startup type '{0}' is not supported by {1}.</value>
   </data>
 </root>
diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
index 43dcb82a312..90b46376dc9 100644
--- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
+++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
@@ -61,6 +61,7 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows"
     ) {
         param($parameter, $value, $expected)
         $currentService = Get-CimInstance -ClassName Win32_Service -Filter "Name='spooler'"
+        $originalStartupType = (Get-Service -Name spooler).StartType
         try {
             $setServiceCommand = [Microsoft.PowerShell.Commands.SetServiceCommand]::new()
             $setServiceCommand.Name = "Spooler"
@@ -79,7 +80,7 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows"
         }
         finally {
             if ($parameter -eq "StartupType") {
-                $setServiceCommand.StartupType = $currentService.StartMode
+                $setServiceCommand.StartupType = $originalStartupType
             }
             else {
                 $setServiceCommand.$parameter = $currentService.$parameter
@@ -161,16 +162,23 @@ Describe "Set/New-Service cmdlet tests" -Tags "Feature", "RequireAdminOnWindows"
         }
     }
 
-    It "New-Service with bad parameters will fail for '<name>' where '<parameter>' = '<value>'" -TestCases @(
-        @{name = 'credtest'    ; parameter = "Credential" ; value = (
+    It "Using bad parameters will fail for '<name>' where '<parameter>' = '<value>'" -TestCases @(
+        @{cmdlet="New-Service"; name = 'credtest'    ; parameter = "Credential" ; value = (
             [System.Management.Automation.PSCredential]::new("username", 
-            (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force)))
-        },
-        @{name = 'badstarttype'; parameter = "StartupType"; value = "System"},
-        @{name = 'winmgmt'     ; parameter = "DisplayName"; value = "foo"}
+            (ConvertTo-SecureString "PlainTextPassword" -AsPlainText -Force)));
+            errorid = "CouldNotNewService,Microsoft.PowerShell.Commands.NewServiceCommand"},
+        @{cmdlet="New-Service"; name = 'badstarttype'; parameter = "StartupType"; value = "System";
+            errorid = "CouldNotNewService,Microsoft.PowerShell.Commands.NewServiceCommand"},
+        @{cmdlet="New-Service"; name = 'winmgmt'     ; parameter = "DisplayName"; value = "foo";
+            errorid = "CouldNotNewService,Microsoft.PowerShell.Commands.NewServiceCommand"},
+        @{cmdlet="Set-Service"; name = 'winmgmt'     ; parameter = "StartupType"; value = "Boot";
+            errorid = "CouldNotSetService,Microsoft.PowerShell.Commands.SetServiceCommand"}
     ) {
-        param($name, $parameter, $value)
-        $parameters = @{$parameter = $value; Name = $name; Binary = "$PSHOME\powershell.exe"; ErrorAction = "Stop"}
-        { New-Service @parameters } | ShouldBeErrorId "CouldNotNewService,Microsoft.PowerShell.Commands.NewServiceCommand"
+        param($cmdlet, $name, $parameter, $value, $errorid)
+        $parameters = @{$parameter = $value; Name = $name; ErrorAction = "Stop"}
+        if ($cmdlet -eq "New-Service") {
+            $parameters += @{Binary = "$PSHOME\powershell.exe"}; 
+        }
+        { & $cmdlet @parameters } | ShouldBeErrorId $errorid
     }
 }

From 25b43a6bb416d625f1b3bc946b9878057fef549f Mon Sep 17 00:00:00 2001
From: "Steve Lee [MSFT]" <slee@microsoft.com>
Date: Tue, 12 Sep 2017 23:29:04 -0700
Subject: [PATCH 5/6] [feature] address PR feedback

---
 .../commands/management/Service.cs                         | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
index e0c7118faf2..df1aa53e0ab 100644
--- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
+++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
@@ -1683,7 +1683,7 @@ protected override void ProcessRecord()
                         {
                             DWORD dwStartType = NativeMethods.SERVICE_NO_CHANGE;
                             if ((ServiceStartMode)(-1) != StartupType &&
-                                !NativeMethods.GetNativeStartupType(StartupType, out dwStartType))
+                                !NativeMethods.TryGetNativeStartupType(StartupType, out dwStartType))
                             {
                                 WriteNonTerminatingError(StartupType.ToString(), "Set-Service", Name,
                                     new ArgumentException(), "CouldNotSetService",
@@ -1992,8 +1992,7 @@ protected override void BeginProcessing()
                         ErrorCategory.PermissionDenied);
                     return;
                 }
-                DWORD dwStartType = NativeMethods.SERVICE_NO_CHANGE;
-                if (! NativeMethods.GetNativeStartupType(StartupType, out dwStartType))
+                if (!NativeMethods.TryGetNativeStartupType(StartupType, out DWORD dwStartType))
                 {
                     WriteNonTerminatingError(StartupType.ToString(), "New-Service", Name,
                         new ArgumentException(), "CouldNotNewService",
@@ -2403,7 +2402,7 @@ public static extern bool QueryInformationJobObject(SafeHandle hJob, int JobObje
         /// <returns>
         /// If a supported StartupType is provided, funciton returns true, otherwise false.
         /// </returns>
-        internal static bool GetNativeStartupType(ServiceStartMode StartupType, out DWORD dwStartType)
+        internal static bool TryGetNativeStartupType(ServiceStartMode StartupType, out DWORD dwStartType)
         {
             bool success = true;
             dwStartType = NativeMethods.SERVICE_NO_CHANGE;

From 7b86b32c1ea4b8fafa53cd653463efec7479637d Mon Sep 17 00:00:00 2001
From: "Steve Lee [MSFT]" <slee@microsoft.com>
Date: Wed, 13 Sep 2017 09:19:26 -0700
Subject: [PATCH 6/6] [feature] made StartupType(-1) equivalent to Win32
 SERVICE_NO_CHANGE and moved to TryGetNativeStartupType

---
 .../commands/management/Service.cs                          | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
index df1aa53e0ab..8d9ea2a0ee2 100644
--- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
+++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
@@ -1682,8 +1682,7 @@ protected override void ProcessRecord()
                             || (ServiceStartMode)(-1) != StartupType)
                         {
                             DWORD dwStartType = NativeMethods.SERVICE_NO_CHANGE;
-                            if ((ServiceStartMode)(-1) != StartupType &&
-                                !NativeMethods.TryGetNativeStartupType(StartupType, out dwStartType))
+                            if (!NativeMethods.TryGetNativeStartupType(StartupType, out dwStartType))
                             {
                                 WriteNonTerminatingError(StartupType.ToString(), "Set-Service", Name,
                                     new ArgumentException(), "CouldNotSetService",
@@ -2417,6 +2416,9 @@ internal static bool TryGetNativeStartupType(ServiceStartMode StartupType, out D
                 case ServiceStartMode.Disabled:
                     dwStartType = NativeMethods.SERVICE_DISABLED;
                     break;
+                case (ServiceStartMode)(-1):
+                    dwStartType = NativeMethods.SERVICE_NO_CHANGE;
+                    break;
                 default:
                     success = false;
                     break;