Fix test_unaligned_bit_chunk_iterator on aarch64 #1297
Conversation
|
I have tested this on an aarch64 M1 Mac and it now passes 😀 |
Codecov Report
@@ Coverage Diff @@
## master #1297 +/- ##
=======================================
Coverage 83.01% 83.01%
=======================================
Files 180 180
Lines 52715 52731 +16
=======================================
+ Hits 43760 43774 +14
- Misses 8955 8957 +2
Continue to review full report at Codecov.
|
tustvold
force-pushed
the
fix-unaligned-bit-chunk-test
branch
from
1c46241 to
5493bd9
Compare
| @@ -466,9 +466,6 @@ mod tests { | |||
| #[test] | |||
| #[allow(clippy::assertions_on_constants)] | |||
| fn test_unaligned_bit_chunk_iterator() { | |||
| // This test exploits the fact Buffer is at least 64-byte aligned | |||
| assert!(ALIGNMENT > 64); | |||
There was a problem hiding this comment.
The derp here is strong. The test requires that Buffer is 64 bit aligned, not 64 byte aligned 🤦
As an added bonus, even if it did care, this should be >=
| @@ -520,6 +516,13 @@ mod tests { | |||
| assert_eq!(unaligned.suffix(), None); | |||
|
|
|||
| let buffer = Buffer::from(&[0xFF; 14]); | |||
|
|
|||
| // Verify buffer alignment | |||
| let (prefix, aligned, suffix) = unsafe { buffer.as_slice().align_to::<u64>() }; | |||
There was a problem hiding this comment.
Technically align_to is not guaranteed to return the maximal aligned slice. In practice we rely on this in so many places, this is what the current upstream implementation will do, and any other behaviour would be extremely contrived.
|
@nevi-me would you like me to spin up a new RC with this change? Edit: or are you comfortable with releasing 9.0.3 given that this is only a test issue? |
alamb
changed the title
I'm away from my laptop, I didn't see if the failure prevented other crates from running, or if they ran. I'll confirm this evening (in about 4-5 hours). I don't think cutting another RC would be necessary. Thanks @alamb |
Which issue does this PR close?
Closes #1294
Rationale for this change
Test should pass
What changes are included in this PR?
Fix test
Are there any user-facing changes?
No, this only changes a test