-
Notifications
You must be signed in to change notification settings - Fork 8.1k
added tests for primitive types, dictionary type, and enumerable type for convertto-xml #4503
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
added tests for primitive types, dictionary type, and enumerable type for convertto-xml #4503
Conversation
| $x.Objects.Object."#text" | Should BeExactly $i | ||
| } | ||
|
|
||
| It "Serialize dictionary type" { |
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.
If you're using terminology from the source code, then I would call this test
"Serialize ContainerType.Dictionary type"
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.
ok
| $x.Objects.Object.Property[1]."#text" | Should BeExactly "bar" | ||
| } | ||
|
|
||
| It "Serialize enumerable type" { |
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.
Like above; maybe rename the test to "Serialize ContainerType.Enumerable type"
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.
ok
| $fruit2.name = "banana" | ||
| $x = $fruit1,$fruit2 | ConvertTo-Xml | ||
| $x.Objects.Object.Count | Should BeExactly 2 | ||
| $x.Objects.Object[0].Type = "fruit" |
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.
For this line and below:
Are these assignments ("=") meant to be verifications ( | Should ) ?
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.
yeah, that's a mistake
adityapatwardhan
left a comment
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.
Please fix CI errors.
|
CI failing with: Expected string length 5 but was 27. Strings differ at index 0. |
176a129 to
005aa87
Compare
|
@anmenaga @adityapatwardhan feedback addressed |
|
@anmenaga Can you have a look? |
Based on https://codecov.io/gh/PowerShell/PowerShell/src/master/src/Microsoft.PowerShell.Commands.Utility/commands/utility/CustomSerialization.cs
Since I'm going to be on vacation, I'm submitting PR now rather than when I finish going through the whole file