-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes ambiguity of parameter sets in VSTeamUserEntitlement cmdlets #400
Fixes ambiguity of parameter sets in VSTeamUserEntitlement cmdlets #400
Conversation
- added tests for changed parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change shouldn't break existing behavior unless someone was counting on the buggy parameter validation experience to throw.
This change should bring the actual behavior in line with the documented behavior.
Are there any docs that need to change with the parameter set change?
-Email '[email protected]' ` | ||
-Force | ||
|
||
Should -Invoke _callAPI -Exactly -Times 1 -Scope It -ParameterFilter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are three values that are being passed up to the API in each of these requests. The only thing the function does is determine if there's a new value to apply or just pass back the current value.
This test should assert that the submitted update object has the expected license type since that isn't changing per the update call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smurawski I really tried to understand what you are saying. But I don't get what I should test in your opinion. The licenses are asserted, arent't they?
the cdmdlet does not have return so I can't test the correct return values. I will go ahead with the bug fix, but maybe you could still comment on this and I can improve the test later.
$resource -eq 'userentitlements' -and | ||
$version -eq $(_getApiVersion MemberEntitlementManagement) | ||
} | ||
} | ||
|
||
It 'by id should update a user' { | ||
It 'by id without license should update a user' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is missing similar validations to by email without a license
.
Thanks and good point. I gonna check the docs for this. |
fixes Ambiguity or bug in Update-VSTeamUserEntitlement #393
added tests for changed parameter
Update Changelog and Moduleversion