Skip to content
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

Fix tests on big endian architectures #289

Closed
wants to merge 1 commit into from
Closed

Conversation

@antlarr
Copy link

antlarr commented Jan 25, 2017

I noticed these two tests were failing on BE systems
and fixes were committed to other tests with same symptoms
so I applied the same logic here to fix these.

I noticed these two tests were failing on BE systems
and fixes were committed to other tests with same symptoms
so I applied the same logic here to fix these.
@lazka

This comment has been minimized.

Copy link
Member

lazka commented Jan 25, 2017

Thanks. While it looks fine I'm wondering if we shouldn't just force a byte order/bom when writing..

@antlarr

This comment has been minimized.

Copy link
Author

antlarr commented Jan 25, 2017

I'm afraid I don't know the answer to that since I'm not familiar with ID3 tags internals (just learning a bit about them lately). I just noticed these when packaging the latest version of mutagen in the openSUSE build service, which builds also ppc64 packages. In any case, if you want me to test anything on such systems, please tell me and I'll do my best to help.

@lazka

This comment has been minimized.

Copy link
Member

lazka commented Jan 25, 2017

In any case, if you want me to test anything on such systems, please tell me and I'll do my best to help.

thanks, appreciated. I'll have a look later.

lazka added a commit that referenced this pull request Jan 25, 2017
While both variants are valid for ID3 this makes tags the same
on all platforms and reduces the risk that tests break on big endian
machines.
@lazka

This comment has been minimized.

Copy link
Member

lazka commented Jan 25, 2017

I've pushed a potential fix to master: c27b957

@antlarr

This comment has been minimized.

Copy link
Author

antlarr commented Jan 25, 2017

Perfect! I tested your "potential fix" c27b957 on build.opensuse.org/.../python3-mutagen and it builds successfully in every architecture (tests included). At build.opensuse.org/.../python3-mutagen/ppc64 build log you can see the output of the ppc64 build in case you're interested.

I'll close this pull request since it's not needed anymore.

@antlarr antlarr closed this Jan 25, 2017
@lazka

This comment has been minimized.

Copy link
Member

lazka commented Jan 25, 2017

Thanks for testing and packaging mutagen!

@lazka

This comment has been minimized.

Copy link
Member

lazka commented Jan 25, 2017

I'll release a 1.36.2 shortly with this fix.

lazka added a commit that referenced this pull request Jan 25, 2017
While both variants are valid for ID3 this makes tags the same
on all platforms and reduces the risk that tests break on big endian
machines.
@lazka

This comment has been minimized.

Copy link
Member

lazka commented Jan 25, 2017

I'll release a 1.36.2 shortly with this fix.

done

@antlarr

This comment has been minimized.

Copy link
Author

antlarr commented Jan 25, 2017

Thanks! I already submitted the new version to the openSUSE devel project at https://build.opensuse.org/request/show/452416 and it will hopefully be available in Tumbleweed soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.