Adding ValidateNotNullOrEmpty attribute to parameters listed in #2672#2685
Adding ValidateNotNullOrEmpty attribute to parameters listed in #2672#2685lzybkr merged 1 commit intoPowerShell:masterPowerShell/PowerShell:masterfrom dlwyatt:NewValidationAttributesdlwyatt/PowerShell:NewValidationAttributesCopy head branch name to clipboard
Conversation
|
I imagine we'd eventually have tests per cmdlet to keep it organized so I would prefer having individual tests scripts we would eventually build up for complete test coverage |
|
@dlwyatt Agree with @SteveL-MSFT and that's the approach we have used with #2542 and #2545 as well. |
|
Perhaps we should add the relevant comments in the Provider cmdlets why we don't fix them to block accidental changes in future. |
SteveL-MSFT
left a comment
There was a problem hiding this comment.
Get-Service -Name is missing
|
Get-Service was already fixed in 6763854 . |
|
i believe we do need tests against metadata changes. Strictly speaking we should really have tests which ensure that we have the proper parameters and that their configuration (type/attributes/etc) is correct. |
|
@JamesWTruher - are you fine with accepting the PR without tests? If we do add tests, I'd rather see that we test behaviors, e.g. that When I've seen test failures that validated implementation details, the fix was always to change the expectations in the test, which to me means the test wasn't catching anything, it was just duplicating the code in a test. |
Do changes to parameter metadata require tests? If so, should they be located with the tests for each individual command, or would it be sufficient to write a single
Itblock with-TestCasesto hit all of the affected commands in this issue / PR?