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

pytools/test/test_persistent_dict.py::test_{attrs,dataclass}_hashing failures on big-endian #262

Closed
mgorny opened this issue Sep 21, 2024 · 2 comments · Fixed by #285
Closed

Comments

@mgorny
Copy link
Contributor

mgorny commented Sep 21, 2024

I'm sorry that I've missed this previously but looks like cee5027 introduced two test regressions on big-endian:

$ pytest pytools/test/test_persistent_dict.py::test_{attrs,dataclass}_hashing
========================================================= test session starts =========================================================
platform linux -- Python 3.12.3, pytest-8.3.2, pluggy-1.5.0
rootdir: /home/mgorny/pytools
configfile: pyproject.toml
plugins: xdist-3.6.1
collected 2 items                                                                                                                     

pytools/test/test_persistent_dict.py FF                                                                                         [100%]

============================================================== FAILURES ===============================================================
_________________________________________________________ test_attrs_hashing __________________________________________________________

    def test_attrs_hashing() -> None:
        attrs = pytest.importorskip("attrs")
    
        keyb = KeyBuilder()
    
        @attrs.define
        class MyAttrs:
            name: str
            value: int
    
>       assert (keyb(MyAttrs("hi", 1)) == "5b6c5da60eb2bd0f")  # type: ignore[call-arg]
E       AssertionError: assert '626f565387d8b006' == '5b6c5da60eb2bd0f'
E         
E         - 5b6c5da60eb2bd0f
E         + 626f565387d8b006

pytools/test/test_persistent_dict.py:593: AssertionError
_______________________________________________________ test_dataclass_hashing ________________________________________________________

    def test_dataclass_hashing() -> None:
        keyb = KeyBuilder()
    
        @dataclass
        class MyDC:
            name: str
            value: int
    
>       assert keyb(MyDC("hi", 1)) == "d1a1079f1c10aa4f"
E       AssertionError: assert 'e36c7d4306a3670a' == 'd1a1079f1c10aa4f'
E         
E         - d1a1079f1c10aa4f
E         + e36c7d4306a3670a

pytools/test/test_persistent_dict.py:569: AssertionError
======================================================= short test summary info =======================================================
FAILED pytools/test/test_persistent_dict.py::test_attrs_hashing - AssertionError: assert '626f565387d8b006' == '5b6c5da60eb2bd0f'
FAILED pytools/test/test_persistent_dict.py::test_dataclass_hashing - AssertionError: assert 'e36c7d4306a3670a' == 'd1a1079f1c10aa4f'
========================================================== 2 failed in 0.35s ==========================================================

Since we are using the system byte order now, the hashes are different on big-endian and little-endian systems. I suppose this can be fixed either by updating the test to have different expected values depending on sys.byteorder, or by mocking it to force little-endian:

diff --git a/pytools/test/test_persistent_dict.py b/pytools/test/test_persistent_dict.py
index b04d43a..cdddb37 100644
--- a/pytools/test/test_persistent_dict.py
+++ b/pytools/test/test_persistent_dict.py
@@ -1,6 +1,7 @@
 import shutil
 import sys
 import tempfile
+import unittest.mock
 from dataclasses import dataclass
 from enum import Enum, IntEnum
 from typing import Any, Dict, Optional
@@ -566,7 +567,8 @@ def test_dataclass_hashing() -> None:
         name: str
         value: int
 
-    assert keyb(MyDC("hi", 1)) == "d1a1079f1c10aa4f"
+    with unittest.mock.patch("sys.byteorder", "little"):
+        assert keyb(MyDC("hi", 1)) == "d1a1079f1c10aa4f"
 
     assert keyb(MyDC("hi", 1)) == keyb(MyDC("hi", 1))
     assert keyb(MyDC("hi", 1)) != keyb(MyDC("hi", 2))
@@ -590,7 +592,8 @@ def test_attrs_hashing() -> None:
         name: str
         value: int
 
-    assert (keyb(MyAttrs("hi", 1)) == "5b6c5da60eb2bd0f")  # type: ignore[call-arg]
+    with unittest.mock.patch("sys.byteorder", "little"):
+        assert (keyb(MyAttrs("hi", 1)) == "5b6c5da60eb2bd0f")  # type: ignore[call-arg]
 
     assert keyb(MyAttrs("hi", 1)) == keyb(MyAttrs("hi", 1))  # type: ignore[call-arg]
     assert keyb(MyAttrs("hi", 1)) != keyb(MyAttrs("hi", 2))  # type: ignore[call-arg]

Please let me know if I should make a PR doing either of the two.

@inducer
Copy link
Owner

inducer commented Sep 21, 2024

Upon second thought, I think cee5027 was the wrong approach. numpy supports byteswapping, so we should just standardize its representation to a fixed byte order (and revert cee5027) IMO.

@inducer
Copy link
Owner

inducer commented Sep 21, 2024

cc @matthiasdiener

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 a pull request may close this issue.

2 participants