ValvePython / vdf

📜 Package for working with Valve's text and binary KeyValue format
https://pypi.org/project/vdf/
MIT License
165 stars 30 forks source link

BinaryVDF.test_loads_utf16 not passed #33

Open tim77 opened 3 years ago

tim77 commented 3 years ago

Some tests not passed in Fedora 32:

=================================== FAILURES ===================================
__________________________ BinaryVDF.test_loads_utf16 __________________________
self = <tests.test_binary_vdf.BinaryVDF testMethod=test_loads_utf16>
    def test_loads_utf16(self):
>       self.assertEqual({'aaa': b'b\x00b\x00\xff\xffb\x00b\x00'.decode('utf-16le')}, vdf.binary_loads(b'\x05aaa\x00b\x00b\x00\xff\xffb\x00b\x00\x00\x00\x08'))
E       AssertionError: {'aaa': 'bb\uffffbb'} != {'aaa': '戀戀\uffff戀戀'}
E       - {'aaa': 'bb\uffffbb'}
E       ?          ^^      ^^
E       
E       + {'aaa': '戀戀\uffff戀戀'}
E       ?          ^^      ^^
tests/test_binary_vdf.py:188: AssertionError

But tests passed in Fedora 33 and no such issue. Full F32 build log.

Additional information:

rossengeorgiev commented 3 years ago

That could only happen at https://github.com/ValvePython/vdf/blob/master/vdf/__init__.py#L361

Which means that utf-16 on that build host must have been understood as utf-16be (platform dependent), and since the test uses LE there will be a mismatch. Looking at https://kojipkgs.fedoraproject.org//work/tasks/713/60470713/hw_info.log confirms it.

I'd say this is a bug and I suppose the temporary fix is set the decode to utf-16le. I'm yet to see a single example of Valve using this type anywhere. It's all utf-8. Now that I think of it, I suppose all encode/decode should be LE, otherwise it all breaks on a BE system.

smcv commented 6 months ago

utf-16 on that build host must have been understood as utf-16be (platform dependent)

@tim77, if you're reporting a build/test failure on an unusual CPU architecture, please say so up-front? Otherwise maintainers will tend to assume that you were building on x86.

Debian s390x (which is big-endian) is exhibiting the same build failure (after I fixed a packaging bug that meant we weren't actually running the tests as we intended to). I'll send a merge request shortly.