Clean xScript resource and add tests #290

kwirkykat · 2016-12-06T23:12:57Z

This is the cleaned xScript resource for #160.

Unit and integration tests have been added and the documentation for xScript has been updated accordingly.

I also just moved the xWindowsFeature section in the README to where it should be alphabetically in the list since it was originally where Script should go.

This PR contains a change that is also included in #289 which is the Get-AppVeyorAdministrator credential in the CommonTestHelper. I will rebase and resolve the conflict after one of these two PRs is merged.

@mbreakey3 and/or @TravisEz13 Please review.

This change is

TravisEz13 · 2016-12-06T23:29:17Z

Tests/CommonTestHelper.psm1

+    [CmdletBinding()]
+    param ()
+
+    if ($null -eq $script:appVeyorAdministratorCredential)


Set-StrictMode -Version latest is not on?

@TravisEz13 It's turned on in #289 where I made the main changes to the CommonTestHelper. You can see it is included on this line for the changes included in that PR. This exact same function is there and passes StrictMode.

I did not want to include all the changes to the CommonTestHelper here. I only included Get-AppVeyorAdministratorCredential here because it is needed to get a valid credential to test running a script under.

Just FYI, this will break when they merge

Fixed the variable initialization

mbreakey3 · 2016-12-07T22:19:27Z

Reviewed 8 of 11 files at r1, 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 18 unresolved discussions.

README.md, line 230 at r2 (raw file):

* **[String] GetScript** _(Key)_: A string that can be used to create a PowerShell script block that retrieves the current state of the resource. This script block runs when the Get-DscConfiguration cmdlet is called. This script block should return a hash table containing one key named Result with a string value.
* **[String] SetScript** _(Key)_: A string that can be used to create PowerShell script block that sets the resource to the desired state. This script block runs conditionally when the Start-DscConfiguration cmdlet is called. The TestScript  script block will run first. If the TestScript block returns False, this script block will run. If the TestScript block returns True, this script block will not run. This script block should not return.

'A string that can be used to create a PowerShell script block...'
Also, is there an extra space between 'The TestScript' and 'script block will run first.' ? It's hard to tell.

DSCResources/MSFT_xScriptResource/MSFT_xScriptResource.psm1, line 210 at r2 (raw file):

    $invokeScriptResult = Invoke-Script @invokeScriptParameters

    # If the script is returing multiple objects, then we consider the last object to be the result of script execution.

'...result of the script execution.'

DSCResources/MSFT_xScriptResource/MSFT_xScriptResource.schema.mof, line 6 at r2 (raw file):

{
  [Key, Description("A string that can be used to create a PowerShell script block that retrieves the current state of the resource.")] String GetScript;
  [Key, Description("A string that can be used to create PowerShell script block that sets the resource to the desired state.")] String SetScript;

'..to create a PowerShell script block..'

DSCResources/MSFT_xScriptResource/en-US/MSFT_xScriptResource.schema.mfl, line 5 at r2 (raw file):

{
    [Key,Description("A string that can be used to create a PowerShell script block that retrieves the current state of the resource.") : Amended] String GetScript;
    [Key,Description("A string that can be used to create PowerShell script block that sets the resource to the desired state.") : Amended] String SetScript;

'...to create a PowerShell script block...'

Tests/CommonTestHelper.psm1, line 784 at r2 (raw file):

    if ($null -eq $script:appVeyorAdministratorCredential)
    {
        $randomObj = New-Object System.Random

New-Object -TypeName System.Random

Tests/CommonTestHelper.psm1, line 785 at r2 (raw file):

    {
        $randomObj = New-Object System.Random
        $password = ""

single quotes

Tests/CommonTestHelper.psm1, line 786 at r2 (raw file):

        $randomObj = New-Object System.Random
        $password = ""
        1..(Get-Random -Minimum 15 -Maximum 126) | ForEach { $password = $password + [char]$randomObj.next(45,126) }

[Char]

Tests/CommonTestHelper.psm1, line 793 at r2 (raw file):

        $objUser = [ADSI]("WinNT://$($env:computerName)/$username")
        $null = $objUser.SetPassword($password)
        [Microsoft.Win32.Registry]::SetValue("HKEY_LOCAL_MACHINE\Software\Microsoft\Windows NT\CurrentVersion\Winlogon", "DefaultPassword", $password)

single quotes

Tests/Integration/MSFT_xScriptResource.Integration.Tests.ps1, line 44 at r2 (raw file):

        }

        Context 'Get, set, and test scripts specified and Credential not specified' {

'Get, Set, and Test...'

Tests/Integration/MSFT_xScriptResource.Integration.Tests.ps1, line 70 at r2 (raw file):

            }

            It 'Should have created test file' {

'Should have created the test file'

Tests/Integration/MSFT_xScriptResource.Integration.Tests.ps1, line 79 at r2 (raw file):

        }

        Context 'Get, set, and test scripts specified and Credential specified' {

'Get, Set, Test..'

Tests/Integration/MSFT_xScriptResource.Integration.Tests.ps1, line 116 at r2 (raw file):

            }

            It 'Should have created test file' {

'...the test file'

Tests/Unit/MSFT_xScriptResource.Tests.ps1, line 1 at r2 (raw file):

# All tests with credentials will be skipped

It doesn't look like these are being skipped anymore

Tests/Unit/MSFT_xScriptResource.Tests.ps1, line 22 at r2 (raw file):

        $secureTestPassword = ConvertTo-SecureString -String $testPassword -AsPlainText -Force

        $script:testCredenital = New-Object -TypeName 'System.Management.Automation.PSCredential' -ArgumentList @( $testUsername, $secureTestPassword)

no space needed between '@(' and '$testUsername'. Or, add a space before the last parenthesis

Tests/Unit/MSFT_xScriptResource.Tests.ps1, line 90 at r2 (raw file):

                    $null = Get-TargetResource @getTargetResourceParameters
                    Assert-MockCalled -CommandName 'Invoke-Script' -ParameterFilter { $null -eq (Compare-Object -ReferenceObject $expectedScriptBlock.Ast -DifferenceObject $ScriptBlock.Ast) } -Times 1 -Scope 'It'

I know it's kind of a pain, but can you break up the 'Assert-MockCalled lines'

Tests/Unit/MSFT_xScriptResource.Tests.ps1, line 283 at r2 (raw file):

            Mock -CommandName 'Invoke-Script' -MockWith { return @( (-not $expectedBoolean), $expectedBoolean ) }

            Context 'Specified get script returns multiple booleans' {

'Specified test script'

Tests/Unit/MSFT_xScriptResource.Tests.ps1, line 309 at r2 (raw file):

            Mock -CommandName 'Invoke-Script' -MockWith { return 'Value1' }

            Context 'Specified get script returns a string' {

'Specified test script...'

Comments from Reviewable

kwirkykat · 2016-12-08T19:24:57Z

Review status: 4 of 11 files reviewed at latest revision, 18 unresolved discussions.

README.md, line 230 at r2 (raw file):