Skip to content

Big Endian: add daily workflow UT job and fix UTs#3330

Open
rainsupreme wants to merge 6 commits intovalkey-io:unstablefrom
rainsupreme:big-endian-uts
Open

Big Endian: add daily workflow UT job and fix UTs#3330
rainsupreme wants to merge 6 commits intovalkey-io:unstablefrom
rainsupreme:big-endian-uts

Conversation

@rainsupreme
Copy link
Contributor

Big endian support on Valkey is "best effort" and not guaranteed, but we haven't been doing any regular testing at all afaik. This PR adds a job to the daily workflow to run UTs on an emulated big endian platform. Integration tests failed excessively because of how slow emulation is.

I fixed several problems with tests and improved UT coverage of key points where endian byte order matters - and fwiw I didn't find any bugs. I think the main coverage gap remaining after this is RDB serialization (maybe little endian <-> big endian round trips?)

And why did I do this? I wrote a couple lines of endian-specific code for #3166 and wanted to test it.

@codecov
Copy link

codecov bot commented Mar 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.97%. Comparing base (fc217fc) to head (4078448).

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3330      +/-   ##
============================================
- Coverage     74.98%   74.97%   -0.01%     
============================================
  Files           129      129              
  Lines         71553    71553              
============================================
- Hits          53652    53648       -4     
- Misses        17901    17905       +4     
Files with missing lines Coverage Δ
src/server.h 100.00% <ø> (ø)

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Integration tests (TCL-based) are unreliable under QEMU emulation due to
timing issues

Signed-off-by: Rain Valentine <[email protected]>
Signed-off-by: Rain Valentine <[email protected]>
Signed-off-by: Rain Valentine <[email protected]>
Signed-off-by: Rain Valentine <[email protected]>
Signed-off-by: Rain Valentine <[email protected]>
@Scottcjn
Copy link

Scottcjn commented Mar 9, 2026

Great to see big-endian getting proper CI attention in Valkey!

We run an IBM POWER8 S824 (16-core, 128 threads, 512GB RAM) as a daily driver for LLM inference and blockchain attestation — native big-endian ppc64. Happy to run the full Valkey test suite on real hardware if that would be useful for validating beyond QEMU emulation.

We also have Power Mac G5s (PPC64 big-endian, Mac OS X / Linux) and PowerPC G4s (32-bit big-endian) if older architectures are of interest.

Real hardware catches timing-dependent and memory-ordering issues that emulation often misses. Let us know if we can help.

@madolson
Copy link
Member

madolson commented Mar 9, 2026

@Scottcjn That would be amazing if you could help run some tests! We at some point tried to acquire some IBM hardware for testing and hit some snags. In the past we've discussed that IBM is sort of tier 2 supported, we will accept fixes but since we can't reliably test on it it's hard to say it's officially supported.

@zuiderkwast zuiderkwast added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Mar 9, 2026
@rainsupreme
Copy link
Contributor Author

Here's a run of just the new job: https://github.com/rainsupreme/valkey/actions/runs/22814475783

@rainsupreme rainsupreme requested a review from madolson March 11, 2026 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants