xBitlockerCommon: Add remaining Unit tests for xBitlockerCommon #28
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #28 +/- ##
=======================================
Coverage 26.08% 26.08%
=======================================
Files 3 3
Lines 92 92
=======================================
Hits 24 24
Misses 68 68
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Reviewed 5 of 6 files at r1.
Reviewable status: 5 of 6 files reviewed, 9 unresolved discussions (waiting on @mhendric)
DSCResources/MSFT_xBLAutoBitlocker/MSFT_xBLAutoBitlocker.psm1, line 298 at r1 (raw file):
param ( [parameter(Mandatory = $true)]
Parameter and the rest of these do not have the parameter decoration. There is ton of white space here, so check all of your functions.
Misc/xBitlockerCommon.psm1, line 814 at r1 (raw file):
( [Parameter(Mandatory = $true)] $PSBoundParametersIn,
Needs a type.
Misc/xBitlockerCommon.psm1, line 860 at r1 (raw file):
( [Parameter(Mandatory = $true)] $PSBoundParametersIn,
Needs type
Misc/xBitlockerCommon.psm1, line 873 at r1 (raw file):
if ($ParamsToKeep.Count -gt 0 -and $ParamsToRemove.Count -gt 0) { throw 'Remove-FromPSBoundParametersUsingHashtable does not support using both ParamsToKeep and ParamsToRemove'
Can you add a parameter set to the function and let PS do this work for you?
Misc/xBitlockerCommon.psm1, line 880 at r1 (raw file):
$ParamsToKeep = $ParamsToKeep.ToLower() $lowerParamsToKeep = Convert-StringArrayToLowerCase -Array $ParamsToKeep
You can let PS do this for you with $lowerParamsToKeep = $lowerParamsToKeep.ToLower()
Misc/xBitlockerCommon.psm1, line 895 at r1 (raw file):
foreach ($param in $ParamsToRemove) { $PSBoundParametersIn.Remove($param) | Out-Null
not a big deal, but you can use $null = so that you don't need to setup a pipeline to output to null. small perf improvements add up over time.
Tests/Unit/xBitlockerCommon.tests.ps1, line 121 at r1 (raw file):
TestBitlocker -MountPoint 'C:' -PrimaryProtector 'TpmProtector' | Should -Be $false }
lot of trailing spaces in this file.
Tests/Unit/xBitlockerCommon.tests.ps1, line 542 at r1 (raw file):
Context 'When EnableBitlocker is called, the volume is not yet encrypted, and Enable-Bitlocker does not return a result' { It 'Should throw an exception' { Mock -CommandName Get-BitLockerVolume -Verifiable -MockWith { return $decryptedOSBLV }
You should be able to use the module param in the mock so that you don't have to have the empty functions at the top.
Tests/Unit/xBitlockerCommon.tests.ps1, line 733 at r1 (raw file):
} <#
dead code should be removed
There was a problem hiding this comment.
Reviewable status: 5 of 6 files reviewed, 9 unresolved discussions (waiting on @athaynes)
DSCResources/MSFT_xBLAutoBitlocker/MSFT_xBLAutoBitlocker.psm1, line 298 at r1 (raw file):
Previously, athaynes (Adam Haynes) wrote…
Parameter and the rest of these do not have the parameter decoration. There is ton of white space here, so check all of your functions.
This is fixed in #27, as is unnecessary whitespace throughout the module.
Misc/xBitlockerCommon.psm1, line 814 at r1 (raw file):
Previously, athaynes (Adam Haynes) wrote…
Needs a type.
Agreed. Although I've tried setting the type to System.Collection.Hashtable (which PSBoundParameters is), and for some reason it prevents modifications of the original item (almost like it's getting passed by Value instead of by Reference). I will try with System.Object and see if it behaves better.
Misc/xBitlockerCommon.psm1, line 860 at r1 (raw file):
Previously, athaynes (Adam Haynes) wrote…
Needs type
Agreed. Although I've tried setting the type to System.Collection.Hashtable (which PSBoundParameters is), and for some reason it prevents modifications of the original item (almost like it's getting passed by Value instead of by Reference). I will try with System.Object and see if it behaves better.
Misc/xBitlockerCommon.psm1, line 873 at r1 (raw file):
Previously, athaynes (Adam Haynes) wrote…
Can you add a parameter set to the function and let PS do this work for you?
Good suggestion. I'll give it a try.
Misc/xBitlockerCommon.psm1, line 880 at r1 (raw file):
Previously, athaynes (Adam Haynes) wrote…
You can let PS do this for you with $lowerParamsToKeep = $lowerParamsToKeep.ToLower()
Yes, that will be simpler. I'll get it updated.
Misc/xBitlockerCommon.psm1, line 895 at r1 (raw file):
Previously, athaynes (Adam Haynes) wrote…
not a big deal, but you can use $null = so that you don't need to setup a pipeline to output to null. small perf improvements add up over time.
I'll give that a try. As long as it doesn't trigger the unused variables rule from the Script Analyzer...
Tests/Unit/xBitlockerCommon.tests.ps1, line 121 at r1 (raw file):
Previously, athaynes (Adam Haynes) wrote…
lot of trailing spaces in this file.
Should be fixed in #27
Tests/Unit/xBitlockerCommon.tests.ps1, line 542 at r1 (raw file):
Previously, athaynes (Adam Haynes) wrote…
You should be able to use the module param in the mock so that you don't have to have the empty functions at the top.
In my experience, if the machine running the tests doesn't have the module installed with the function to Mock (in this case the Bitlocker module), Mock will fail. Mock only succeeds on functions that are visible at the time of the Mock. I'll give the -Module parameter a try though.
Tests/Unit/xBitlockerCommon.tests.ps1, line 733 at r1 (raw file):
Previously, athaynes (Adam Haynes) wrote…
dead code should be removed
Should be fixed in #27
There was a problem hiding this comment.
Reviewable status: 3 of 6 files reviewed, 9 unresolved discussions (waiting on @athaynes)
Misc/xBitlockerCommon.psm1, line 814 at r1 (raw file):
Previously, mhendric (Mike Hendrickson) wrote…
Agreed. Although I've tried setting the type to System.Collection.Hashtable (which PSBoundParameters is), and for some reason it prevents modifications of the original item (almost like it's getting passed by Value instead of by Reference). I will try with System.Object and see if it behaves better.
Done.
Misc/xBitlockerCommon.psm1, line 860 at r1 (raw file):
Previously, mhendric (Mike Hendrickson) wrote…
Agreed. Although I've tried setting the type to System.Collection.Hashtable (which PSBoundParameters is), and for some reason it prevents modifications of the original item (almost like it's getting passed by Value instead of by Reference). I will try with System.Object and see if it behaves better.
Done.
Misc/xBitlockerCommon.psm1, line 873 at r1 (raw file):
Previously, mhendric (Mike Hendrickson) wrote…
Good suggestion. I'll give it a try.
Done.
Misc/xBitlockerCommon.psm1, line 880 at r1 (raw file):
Previously, mhendric (Mike Hendrickson) wrote…
Yes, that will be simpler. I'll get it updated.
Done.
Misc/xBitlockerCommon.psm1, line 895 at r1 (raw file):
Previously, mhendric (Mike Hendrickson) wrote…
I'll give that a try. As long as it doesn't trigger the unused variables rule from the Script Analyzer...
Done.
Tests/Unit/xBitlockerCommon.tests.ps1, line 542 at r1 (raw file):
Previously, mhendric (Mike Hendrickson) wrote…
In my experience, if the machine running the tests doesn't have the module installed with the function to Mock (in this case the Bitlocker module), Mock will fail. Mock only succeeds on functions that are visible at the time of the Mock. I'll give the -Module parameter a try though.
I tried again, and I can't Mock a function unless Mock knows about the function. So I think we need to keep these empty function declarations in place in the Unit tests for cases where the test machine doesn't have the Bitlocker module. Let me know if you have an alternate suggestion though.
There was a problem hiding this comment.
Reviewed 1 of 6 files at r1, 2 of 2 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
Pull Request (PR) description
Fixes issue where AutoUnlock is not set if requested, if the disk was originally encrypted and AutoUnlock was not used. Also adds remaining Unit Tests for xBitlockerCommon.
What does this fix
Task list
Entry should say what was changed, and how that affects users (if applicable).
and comment-based help.
This change is