Skip to content

Comments

Fix JSON wrong encoding tests on big endian platforms#1781

Merged
lovelydinosaur merged 1 commit intoencode:masterfrom
mgorny:big-endian
Aug 5, 2021
Merged

Fix JSON wrong encoding tests on big endian platforms#1781
lovelydinosaur merged 1 commit intoencode:masterfrom
mgorny:big-endian

Conversation

@mgorny
Copy link
Contributor

@mgorny mgorny commented Aug 5, 2021

Fix test_json_without_specified_encoding_*_error tests on big endian
platforms. The tests wrongly assume that data encoded as "utf-32-be"
can not be decoded as "utf-32". This is true on little endian platforms
but on big endian platforms "utf-32" is equivalent to "utf-32-be".
To avoid the problem, explicitly decode as "utf-32-le", as this should
trigger the expected exception independently of platform's endianness.

Fix test_json_without_specified_encoding_*_error tests on big endian
platforms.  The tests wrongly assume that data encoded as "utf-32-be"
can not be decoded as "utf-32".  This is true on little endian platforms
but on big endian platforms "utf-32" is equivalent to "utf-32-be".
To avoid the problem, explicitly decode as "utf-32-le", as this should
trigger the expected exception independently of platform's endianness.
@mgorny
Copy link
Contributor Author

mgorny commented Aug 5, 2021

I'm sorry for not raising a discussion but for such trivial changes, even if you told me it's all wrong I've spent less time doing it wrong then I'd going through all the formalities.

@lovelydinosaur
Copy link
Contributor

lovelydinosaur commented Aug 5, 2021

Sure thing...

Looking at the codebase I can see that this just makes more sense now, so I'm good with it.
Can you tell us how you bumped into this, or did you just spot it?

(Eg. does it actually fail on a particular platform you were working on, and what was it?)

@lovelydinosaur lovelydinosaur merged commit ee24e67 into encode:master Aug 5, 2021
@mgorny
Copy link
Contributor Author

mgorny commented Aug 5, 2021

One of our ppc64 testers hit the test failure.

@mgorny
Copy link
Contributor Author

mgorny commented Aug 5, 2021

(with the Gentoo package)

@mgorny mgorny deleted the big-endian branch August 5, 2021 16:10
@thesamesam
Copy link

For reference, this was bug 802195 in Gentoo. We have a bunch of big (and little) endian platforms we test popular packages on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants