ticketmaster / poshspec

Infrastructure Testing DSL running in Pester
MIT License
183 stars 32 forks source link

[Bug] Service function prompts for 'Should' input #21

Closed megamorf closed 5 years ago

megamorf commented 8 years ago

Hey guys,

I'd like to report an issue that I ran into when using the service function of poshspec. This is my test block:

Describe 'Appsense is installed and configured' {
    Context 'Appsense Agents are running' {
        Service 'AppSense Application Manager Agent' { Should Be Running }
        Service 'AppSense Client Communications Agent' { Should Be Running }
        Service 'AppSense EmCoreService' { Should Be Running }
        Service 'AppSense Performance Manager Agent' { Should Be Running }
        Service 'AppSense Watchdog Service' { Should Be Running }
    }
}

Now instead of executing the tests it interactively prompts me for should input, that is my prompt shows this and hangs there forever:

Describing Appsense is installed and configured
   Context Appsense Agents are running
Cmdlet Service at command pipeline position 1
Supply values for the following parameters:
Should: 

Environment: Server 2012 R2 Pester v3.4.1 Poshspec v2.1.12 PS v5.0.10586.117

devblackops commented 8 years ago

This is because the Service function expects an additional parameter specifying the property to test against.

Something like this will work:

Describe 'Appsense is installed and configured' {
    Context 'Appsense Agents are running' {
        Service 'AppSense Application Manager Agent' status { Should Be Running }
    }
}

That leads to the question of whether the function should handle this scenario and default to testing the status of the service is no property is specified. What do you think @cdhunt? This would be a simple change to the function.

megamorf commented 8 years ago

I believe it makes sense to be able to check a few more properties such as path, startup type and recovery options.

The wiki and the function's help omit the status parameter name which is why I ran into this issue in the first place (I haven't had the time to check the source code myself).

michaeltlombardi commented 7 years ago

@megamorf I believe you can test for other properties that you can retrieve with Get-Service (Display Name, StartType, CanStop, etc) - however, some, like Path are returned by Get-CimInstance -ClassName Win32_Service

megamorf commented 7 years ago

Yeah, but wouldn't it make sense to construct an object under the hood that has more details than the regular ServiceController object returned by Get-Service? Essentially the user should be able to test against as many settings as possible.

michaeltlombardi commented 7 years ago

The only problem with making that change now is that it could, potentially, break existing tests:

PS C:\> get-service | select -first 1 | fl *

Name                : AdobeARMservice
RequiredServices    : {}
CanPauseAndContinue : False
CanShutdown         : False
CanStop             : True
DisplayName         : Adobe Acrobat Update Service
DependentServices   : {}
MachineName         : .
ServiceName         : AdobeARMservice
ServicesDependedOn  : {}
ServiceHandle       : SafeServiceHandle
Status              : Running
ServiceType         : Win32OwnProcess
StartType           : Automatic
Site                :
Container           :
PS C:\> get-ciminstance -ClassName win32_service | select -first 1 | fl *

Name                    : AdobeARMservice
Status                  : OK
ExitCode                : 0
DesktopInteract         : False
ErrorControl            : Ignore
PathName                : "C:\Program Files (x86)\Common Files\Adobe\ARM\1.0\armsvc.exe"
ServiceType             : Own Process
StartMode               : Auto
Caption                 : Adobe Acrobat Update Service
Description             : Adobe Acrobat Updater keeps your Adobe software up to date.
InstallDate             :
CreationClassName       : Win32_Service
Started                 : True
SystemCreationClassName : Win32_ComputerSystem
SystemName              : MMXL2410KX5
AcceptPause             : False
AcceptStop              : True
DisplayName             : Adobe Acrobat Update Service
ServiceSpecificExitCode : 0
StartName               : LocalSystem
State                   : Running
TagId                   : 0
CheckPoint              : 0
DelayedAutoStart        : False
ProcessId               : 2388
WaitHint                : 0
PSComputerName          :
CimClass                : root/cimv2:Win32_Service
CimInstanceProperties   : {Caption, Description, InstallDate, Name...}
CimSystemProperties     : Microsoft.Management.Infrastructure.CimSystemProperties

Notice that the status property still exists but is different, CanStop becomes AcceptStop, etc.

Since most of the already-written tests will be checking for status, this could potentially break them pretty badly.

An alternative would be to add a Qualifier parameter and let people choose between service objects and the CIM instance as below, where the default option is to return the results of Get-Service so as not to break existing tests.

Describe 'Appsense is installed and configured' {
    Context 'Appsense Agents are running' {
        Service 'AppSense Application Manager Agent' status { Should Be 'Running' }
        Service 'AppSense Application Manager Agent' status AsService { Should Be 'Running' }
        Service 'AppSense Application Manager Agent' status AsCim { Should Be 'OK' }
    }
}
michaeltlombardi commented 7 years ago

Actually, thinking about it now, you should use the CimObject test, not the hacky thing I suggested above; Then your test would look like:

Describe 'Appsense is installed and configured' {
    Context 'Appsense Agents are running' {
        Service 'AppSense Application Manager Agent' status { Should Be 'Running' }
        CimObject Win32_Service Status { Should Be 'OK' }
    }
}
megamorf commented 7 years ago

@michaeltlombardi I've just had a look at the CimObject function and the problem is that it's currently lacking a filter property. It only works with CIM classes that return singletons, e.g.:

PS C:\Users\User> describe 'example' { CimObject Win32_Computersystem Manufacturer { Should Be 'LENOVO' }}
Describing example
 [+] CimObject property 'Manufacturer' for 'Win32_Computersystem' Should Be 'LENOVO' 106ms

Whereas classes that return multiple instances are going to break the CimObject function, e.g. your example:

PS C:\Users\User> describe 'example' { CimObject Win32_Service Status { Should Be 'OK' }}
Describing example
 [-] CimObject property 'Status' for 'Win32_Service' Should Be 'OK' 6.25s
   Expected string length 2 but was 7. Strings differ at index 0.
   Expected: {OK}
   But was:  {UNKNOWN}
   -----------^
michaeltlombardi commented 7 years ago

That's probably something that needs fixing then. Looks like a Qualifier would need to be added to the CimObject test.

DexterPOSH commented 7 years ago

@michaeltlombardi @megamorf I was going through this issue and thought this might interest you as well. Have sort of incomplete implementation of PoshSpec in my fork, where it accepts either a string or a hashtable as the input to the Target parameter.

This allows me to solve the above issue by using below :

Describe 'CIMObjects' {
    CimObject Win32_OperatingSystem SystemDirectory { Should Be C:\WINDOWS\system32 }
    CimObject @{ClassName='Win32_service';Filter="Name='WinRm'"} Status  {Should be 'Ok'} 
}

You can take a look in my branch and let me know how this looks.

cdhunt commented 7 years ago

That's a great idea, @DexterPOSH.

DexterPOSH commented 7 years ago

@cdhunt Cool, I will work more on updating some of the public types and create a pull request.

cdhunt commented 7 years ago

Getting this back on topic. The behavior @megamorf described is not the expected behavior. The Parameter Sets are not binding as expected. It should default to checking the Status property if nothing is provided.