Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Serializer] Xml encoder throws exception for valid data#21671
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ca8c279 to65280e9Compare* add tests for bool and object encoding* fix encoding for object in array and field
| */ | ||
| private$encoder; | ||
| /** |
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 comment is useless (the IDE will be able to infer the type using the default value).
| } | ||
| /** | ||
| * @test |
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 block is useless (same below).
| */ | ||
| publicfunctiontestEncodeXmlWithBoolValue() | ||
| { | ||
| $expectedXml ='<?xml version="1.0"?> |
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.
You may use the heredoc syntax (the IDE will be able to highlight the snippet). (Just suggestion, not mandatory).
| <response><foo>1</foo><bar>0</bar></response> | ||
| '; | ||
| $actualXml =$this->encoder->encode(array('foo' =>true,'bar' =>false),'xml'); |
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.
I don't get why this is expected? IMO it should bearray('foo' => '1', 'bar' => '0') unless explicitly defined as bool in a XML schema.
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 is an actual behavior of XmlEncode, to cast bool to int. I am not sure that i should fix this non explicit behaviour, so i am just cover that with test.
| namespaceSymfony\Component\Serializer\Tests\Encoder; | ||
| useDateTime; |
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 classes in the global namespace, we don't import them (use\DateTime directly).
| namespaceSymfony\Component\Serializer\Tests\Encoder; | ||
| usePHPUnit\Framework\TestCase; | ||
| useSymfony\Component\Serializer\Tests\Fixtures\Dummy; |
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 revert any change related to ordering: our policy is to addnew lines in alpha order, but keep old ones as is - to help for merges between branches.
fabpot commentedMar 1, 2017
@dunglas Is this one ok for you? |
dunglas commentedMar 5, 2017
👍 |
fabpot commentedMar 5, 2017
Thank you@gr1ev0us. |
…gr1ev0us)This PR was squashed before being merged into the 2.7 branch (closes#21671).Discussion----------[Serializer] Xml encoder throws exception for valid data| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#21617| License | MIT| Doc PR | None#21617 Xml encoder throws exception for valid data- add tests for bool and object encoding- fix encoding for object in array and fieldCommits-------5c2d4c6 [Serializer] Xml encoder throws exception for valid data
Uh oh!
There was an error while loading.Please reload this page.
#21617 Xml encoder throws exception for valid data