BREAKING CHANGE: xWindowsOptionalFeature: Update to a high quality resource (Fixes #229) #237

kwirkykat · 2016-09-14T20:05:20Z

There was nothing to merge from the in-box WindowsOptionalFeature for #160 so instead, I went ahead and updated it to a high quality resource.

PSSA is clean on MSFT_xWindowsOptionalFeature.psm1.
Pester is showing unit test coverage at 95%.
Integration tests cover main functionality.
A clearly documented example is provided.
MSFT_xWindowsOptionalFeature.psm1 meets the DSC Resource Kit style guidelines.
All documentation here on GitHub has been updated for this resource.

There is one significant change in this PR which is that the Source parameter has been removed from this resource. This fixes #229. The parameter was not implemented at all in the resource, was not mandatory, and is not a key. However, if you did provide that parameter anyways in a script, those scripts will now break. So I will still mark this as a breaking change.

I also updated the common resource helper with a function to properly import localized strings. (@PlagueHO this is a function to contain the small snippet of localization code you normally add at the beginning of files)

This change is

kwirkykat · 2016-09-14T20:48:05Z

I added a small update after testing with Set-StrictMode -Version 4 which should simulate running the resource in WMF 4.

mbreakey3 · 2016-09-17T00:00:15Z

Reviewed 6 of 11 files at r1, 2 of 2 files at r2, 2 of 3 files at r3.
Review status: 9 of 12 files reviewed at latest revision, 5 unresolved discussions.

DSCResources/MSFT_xWindowsOptionalFeature/MSFT_xWindowsOptionalFeature.psm1, line 90 at r3 (raw file):

    .PARAMETER LogPath
        The path to the log file to log this operation.
        There is not default value, but if not set, the log will appear at

'There is no default...'

DSCResources/MSFT_xWindowsOptionalFeature/en-US/MSFT_xWindowsOptionalFeature.strings.psd1, line 4 at r3 (raw file):

ConvertFrom-StringData @'
DismNotAvailable = PowerShell module DISM could not be imported.

tab this string content in

DSCResources/MSFT_xWindowsOptionalFeature/en-US/MSFT_xWindowsOptionalFeature.strings.psd1, line 8 at r3 (raw file):

ElevationRequired = This resource must run as an Administrator.
ValidatingPrerequisites = Validating resource prerequisites...
CouldNotCovertFeatureState = Could not convert feature state '{0}' into Absent or Present.

'CouldNotConvert...' instead of 'Covert'

Tests/Unit/MSFT_xWindowsOptionalFeature.Tests.ps1, line 61 at r3 (raw file):

                Mock Dism\Get-WindowsOptionalFeature { $FeatureName -eq $script:testFeatureName } -MockWith { return $script:fakeEnabledFeature }

                It 'Should return a Boolean' {

? this test? is it really helpful?

Tests/Unit/MSFT_xWindowsOptionalFeature.Tests.ps1, line 243 at r3 (raw file):

                }

                It 'Should throw when the DISM module is not available' {

Is it possible to specify what is thrown - as this test will pass no matter what's thrown. You could also check that the mock is called

Comments from Reviewable

kwirkykat · 2016-09-20T00:31:01Z

Review status: 8 of 12 files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

DSCResources/MSFT_xWindowsOptionalFeature/MSFT_xWindowsOptionalFeature.psm1, line 90 at r3 (raw file):