Fix module specification hashtable in ModuleSpecification.Tests.ps1#7663
Conversation
|
@sethvs The broken names was created specially for test "Cannot create from invalid module hashtables". |
|
@iSazonov @{TestName = " Now, with Name instead of ModuleName, the code that checks the hashtable throws errors because of the wrong field - Name, instead of the actual error, that should be tested. For example, the first testcase: $ModuleSpecification = @{ Name = "BadVersionModule"; ModuleVersion = "3.1"; RequiredVersion = "3.1" }
[Microsoft.PowerShell.Commands.ModuleSpecification]::new($ModuleSpecification)should actually be: $ModuleSpecification = @{ ModuleName = "BadVersionModule"; ModuleVersion = "3.1"; RequiredVersion = "3.1" }
[Microsoft.PowerShell.Commands.ModuleSpecification]::new($ModuleSpecification) |
| @{ | ||
| TestName = "BadType" | ||
| ModuleSpecification = @{ Name = "BadTypeModule"; RequiredVersion = "Hello!" } | ||
| ModuleSpecification = @{ ModuleName = "BadTypeModule"; RequiredVersion = "Hello!" } |
There was a problem hiding this comment.
To make the tests more effective we modify L255 to be something like below for the specific expected failure. This will ensure such failures are not hidden due to faulty test code.
Should -Throw -ErrorId ArgumentExceptionThere was a problem hiding this comment.
Regarding first and third tests:
@{TestName = "Version+RequiredVersion"
ModuleSpecification = @{ ModuleName = "BadVersionModule"; ModuleVersion = "3.1"; RequiredVersion = "3.1" }}
@{TestName = "BadField"
ModuleSpecification = @{ ModuleName = "StrangeFieldModule"; RequiredVersion = "7.4"; Duck = "1.2" }}
They return ArgumentException, just like when we use Name instead of ModuleName in the hastable, but I agree, that specifying expected ErrorId is a good addition.
|
@sethvs Thanks for your contribution. I have left a comment to make the test even more effective. |
rjmholt
left a comment
There was a problem hiding this comment.
Looks good. Thanks for fixing this. @adityapatwardhan's suggestion is quite right -- a more specific test around the error would have prevented this problem, so it would be good to include here
PR Summary
Change Name to ModuleName in module specification hashtable.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests