Big Endian: add daily workflow UT job and fix UTs#3330
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #3330 +/- ##
============================================
+ Coverage 76.42% 76.70% +0.28%
============================================
Files 159 162 +3
Lines 80113 80608 +495
============================================
+ Hits 61225 61831 +606
+ Misses 18888 18777 -111
🚀 New features to boost your workflow:
|
83aa7a0 to
4078448
Compare
|
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. |
|
@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. |
|
Here's a run of just the new job: https://github.com/rainsupreme/valkey/actions/runs/22814475783 |
enjoy-binbin
left a comment
There was a problem hiding this comment.
Thanks, please fix the conflict.
Integration tests (TCL-based) are unreliable under QEMU emulation due to timing issues Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Rain Valentine <rsg000@gmail.com>
4078448 to
ac228e6
Compare
|
There is one issue where this doesn't skip gtest in the old branches during the weekly runs. |
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?) There are couple lines of endian-specific code for #3166 and this change can test it. Signed-off-by: Rain Valentine <rsg000@gmail.com>
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.