Skip to content

TEST: build the ZIP fixture little-endian in the local-header test#114

Merged
skoudoro merged 2 commits into
tee-ar-ex:masterfrom
Scottcjn:fix/bigendian-zip-test-fixture
Jun 13, 2026
Merged

TEST: build the ZIP fixture little-endian in the local-header test#114
skoudoro merged 2 commits into
tee-ar-ex:masterfrom
Scottcjn:fix/bigendian-zip-test-fixture

Conversation

@Scottcjn

Copy link
Copy Markdown

Fixes #113test_load_zip_with_local_header_extra_field fails on s390x (and any big-endian host).

The test hand-builds a ZIP fixture and wrote the positions / offsets arrays with .tobytes(), which serializes in native byte order. The TRX format stores arrays little-endian on disk (_get_dtype_little_endian / _ensure_little_endian), and the loader decodes them little-endian. So on a big-endian host the fixture wrote big-endian bytes that the (correct) reader then byte-swapped — the test failed with mismatched streamline values.

This is a test-fixture bug, not a library bug: the library's read and write paths are both consistently little-endian.

Fix

Serialize the fixture arrays explicitly as little-endian — positions.astype("<f4").tobytes() and offsets.astype("<u8").tobytes() — so the hand-built ZIP is a valid TRX file regardless of host endianness. The ZIP headers were already written with explicit little-endian struct.pack("<..."); library code is untouched.

Verification

  • Full test_memmap.py suite passes on x86_64 (83 passed) — no regression.
  • Big-endian behaviour confirmed by simulation: a native .tobytes() of a big-endian array does not match the little-endian on-disk form, while .astype("<f4").tobytes() does.

test_load_zip_with_local_header_extra_field hand-builds a ZIP and wrote
the positions/offsets arrays with `.tobytes()`, which serializes in
native byte order. The TRX format stores arrays little-endian on disk
(see `_get_dtype_little_endian` / `_ensure_little_endian`), and the
reader decodes them little-endian — so on a big-endian host (s390x) the
fixture wrote big-endian bytes that the reader then byte-swapped, and the
test failed with mismatched values.

Serialize the fixture arrays explicitly as little-endian
(`astype("<f4")` / `astype("<u8")`) so the test is valid regardless of
host endianness. Library code is unchanged.

Fixes tee-ar-ex#113

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@codecov

codecov Bot commented Jun 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.64%. Comparing base (18ce0e2) to head (125ed04).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #114   +/-   ##
=======================================
  Coverage   61.64%   61.64%           
=======================================
  Files          13       13           
  Lines        2623     2623           
=======================================
  Hits         1617     1617           
  Misses       1006     1006           
Flag Coverage Δ
unittests 61.64% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@skoudoro skoudoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for catching that @Scottcjn !

Can you just remove the comments, then we can merge.

Comment thread trx/tests/test_memmap.py Outdated
Comment on lines +526 to +527
# TRX stores arrays little-endian on disk; serialize the fixture
# explicitly so it is valid on big-endian hosts too (issue #113).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the comments.

Thanks !

Comment thread trx/tests/test_memmap.py Outdated
@arokem

arokem commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Is this ready to go now, @skoudoro ?

@skoudoro

Copy link
Copy Markdown
Collaborator

Thanks @Scottcjn and thanks @arokem ! merging

@skoudoro skoudoro merged commit 951e7c9 into tee-ar-ex:master Jun 13, 2026
16 checks passed
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.

test_load_zip_with_local_header_extra_field failure on s390x

3 participants