Improve test coverage for CDXML cmdlet infrastructure #4537
Improve test coverage for CDXML cmdlet infrastructure #4537daxian-dbw merged 4 commits intoPowerShell:masterfrom
Conversation
Includes MOF files to create and delete class and instances which are used by the tests
updated create mof file to support associations as well as complex objects updated delete mof file to remove all classes added an enum to cdxml (not yet used) updated psd1 to export new cmdlets for set and new
adityapatwardhan
left a comment
There was a problem hiding this comment.
LGTM with minor comments
| Context "Remove-CimTest cmdlet" { | ||
| BeforeEach { | ||
| Get-CimTest | Remove-CimTest | ||
| 1..4 | %{ New-CimInstance -namespace root/default -class PSCore_Test1 -property @{ |
There was a problem hiding this comment.
Please use ForEach-Object instead of %
There was a problem hiding this comment.
This needs to be addressed.
As stated in #3791, our scripts should not have PSScriptAnalyzer warnings and errors.
| field2 = 0 | ||
| } | ||
| New-CimTest @instanceArgs | ||
| $result = Get-CimInstance -namespace root/default -class PSCore_Test1 | ?{$_.id -eq "telephone"} |
There was a problem hiding this comment.
Use Where-Object instead of ?
| $result.field2 | should be 33 | ||
| } | ||
| } | ||
|
|
| HelpInfoUri = "https://go.microsoft.com/fwlink/?linkid=390832" | ||
| } | ||
|
|
||
|
|
| AliasesToExport = @() | ||
| CmdletsToExport = @() | ||
| FunctionsToExport = @( 'Get-CimTest', 'Remove-CimTest', 'New-CimTest', 'Set-CimTest' ) | ||
| HelpInfoUri = "https://go.microsoft.com/fwlink/?linkid=390832" |
There was a problem hiding this comment.
Do we need this? This probably points to something else.
There was a problem hiding this comment.
probably not - removed
| Describe "Cdxml cmdlets are supported" -Tag CI,RequireAdminOnWindows { | ||
| BeforeAll { | ||
| $skipNotWindows = ! $IsWindows | ||
| if ( $skipNotWindows ) { |
There was a problem hiding this comment.
Why not use a direct $IsWindows test versus a variable?
There was a problem hiding this comment.
I thought it was clearer here -skip (I've seen other tests do exactly the opposite of what was wanted)
| } | ||
| $result = MofComp.exe $deleteMof | ||
| if ( $LASTEXITCODE -ne 0 ) { | ||
| $script:ItSkipOrPending = @{ Pending = $true } |
There was a problem hiding this comment.
Why isn't this an outright failure? It appears to be masking a bug.
There was a problem hiding this comment.
I thought about this, and while it might be a bug in the mof file or a problem with mofcomp, marking a test as pending doesn't get us off the hook - all pending tests must be resolved before we ship (and I do track our pending tests). However, I didn't want to invalidate the build because I couldn't even get to the actual test. I thought it was better to mark the test as pending rather than fail.
| $job = Get-CimTest -id 3 | Remove-CimTest -asjob | ||
| $result = $null | ||
| $i = 0 | ||
| # wait up to 10 seconds, then the test will fail |
There was a problem hiding this comment.
Why isn't Wait-Job -Timeout sufficient?
There was a problem hiding this comment.
ahem, yes, that's much better
-fixed-
If there is a problem with mofcomp execution, the tests will fail rather than being marked as pending (feedback from DanTra) use cmdlet names rather than aliases for foreach-object and where-object Script analyzer report is clean Also remove unneeded empty lines and helpinfouri from psd1 file
|
@dantraMSFT, I believe this is ok now |
|
@dantraMSFT Can you please take another look at this PR? |
address issue #4159
Create custom cim classes (via MOF) and cdxml cmdlets against the new cim class
Create tests for CRUD operations for CDXML cmdlets
Coverage for Microsoft.PowerShell.Cmdletization namespace is nearly 50%