Skip to content

fix set-service failing test #4802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iSazonov merged 6 commits into from
Sep 15, 2017
Merged

fix set-service failing test #4802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
babc348
[feature]
SteveL-MSFT Sep 10, 2017
c67a75e
[feature]
SteveL-MSFT Sep 11, 2017
4bde7fb
[feature]
SteveL-MSFT Sep 11, 2017
94561f4
[feature]
SteveL-MSFT Sep 12, 2017
25b43a6
[feature]
SteveL-MSFT Sep 13, 2017
7b86b32
[feature]
SteveL-MSFT Sep 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 49 additions & 34 deletions src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1684,22 +1682,13 @@ protected override void ProcessRecord()
|| (ServiceStartMode)(-1) != StartupType)
{
DWORD dwStartType = NativeMethods.SERVICE_NO_CHANGE;
Copy link
Collaborator

@iSazonov iSazonov Sep 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do the initialiazation in GetNativeStartupType. So we could remove this and below call by C# 7.0 pattern NativeMethods.GetNativeStartupType(StartupType, out DWORD dwStartType)) .

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Sep 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make that change for New-Service, but for Set-Service, I need to use dwStartType on line 1697 so it has to be initialized.

Copy link
Collaborator

@iSazonov iSazonov Sep 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clafiry. I think we should leave as is.
Closed.

Copy link
Member

@daxian-dbw daxian-dbw Sep 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In New-Service, dwStartType is also used in line 2045 when calling NativeMethods.CreateServiceW.
It turns out the following code works:

static void Blah ()
{
    if (!int.TryParse("1", out int result))
    {
        Console.WriteLine("Failed");
    }

    Console.WriteLine(result);
}

It would be the best if we can do it in a consistent way in those 2 places.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Sep 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

switch (StartupType)
if (!NativeMethods.TryGetNativeStartupType(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,
Expand Down Expand Up @@ -2002,25 +1991,14 @@ protected override void BeginProcessing()
ErrorCategory.PermissionDenied);
return;
}
DWORD dwStartType = NativeMethods.SERVICE_AUTO_START;
switch (StartupType)
if (!NativeMethods.TryGetNativeStartupType(StartupType, out DWORD 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(), "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)
Expand Down Expand Up @@ -2410,6 +2388,43 @@ [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 TryGetNativeStartupType(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;
case (ServiceStartMode)(-1):
dwStartType = NativeMethods.SERVICE_NO_CHANGE;
break;
default:
success = false;
break;
}
return success;
}
}
#endregion NativeMethods
}
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {1}.</value>
</data>
</root>
38 changes: 24 additions & 14 deletions test/powershell/Modules/Microsoft.PowerShell.Management/Set-Service.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -42,7 +45,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"
}
) {
Expand All @@ -58,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"
Expand All @@ -76,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
Expand Down Expand Up @@ -158,17 +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)))
},
# This test case fails due to #4803. Disabled for now.
# @{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
}
}