Test with minimum and maximum seed values

Visual inspection of the 64-bit implementation while working on the 128-bit implementation showed a non-wrapping addition. In debug mode, this would panic, but thankfully release mode would wrap as desired. This enhances the property tests to ensure that seeds of all `0` bits and all `1` bits are explicitly tested.
shepmaster · Nov 1, 2024 · 9237e27 · 9237e27
1 parent 75f9e5f
commit 9237e27
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 24 deletions.
diff --git a/comparison/src/lib.rs b/comparison/src/lib.rs
@@ -13,37 +13,37 @@ mod xxhash32 {
 
     proptest! {
         #[test]
-        fn oneshot_same_as_one_chunk(seed: u32, data: Vec<u8>) {
+        fn oneshot_same_as_one_chunk(seed in seed_32(), data: Vec<u8>) {
             oneshot_same_as_one_chunk_impl(seed, &data)?;
         }
 
         #[test]
-        fn oneshot_same_as_one_chunk_with_an_offset(seed: u32, (data, offset) in vec_and_index()) {
+        fn oneshot_same_as_one_chunk_with_an_offset(seed in seed_32(), (data, offset) in vec_and_index()) {
             oneshot_same_as_one_chunk_impl(seed, &data[offset..])?;
         }
 
         #[test]
-        fn oneshot_same_as_many_chunks(seed: u32, (data, chunks) in data_and_chunks()) {
+        fn oneshot_same_as_many_chunks(seed in seed_32(), (data, chunks) in data_and_chunks()) {
             oneshot_same_as_many_chunks_impl(seed, &data, &chunks)?;
         }
 
         #[test]
-        fn oneshot(seed: u32, data: Vec<u8>) {
+        fn oneshot(seed in seed_32(), data: Vec<u8>) {
             oneshot_impl(seed, &data)?;
         }
 
         #[test]
-        fn oneshot_with_an_offset(seed: u32, (data, offset) in vec_and_index()) {
+        fn oneshot_with_an_offset(seed in seed_32(), (data, offset) in vec_and_index()) {
             oneshot_impl(seed, &data[offset..])?;
         }
 
         #[test]
-        fn streaming_one_chunk(seed: u32, data: Vec<u8>) {
+        fn streaming_one_chunk(seed in seed_32(), data: Vec<u8>) {
             streaming_one_chunk_impl(seed, &data)?;
         }
 
         #[test]
-        fn streaming_one_chunk_with_an_offset(seed: u32, (data, offset) in vec_and_index()) {
+        fn streaming_one_chunk_with_an_offset(seed in seed_32(), (data, offset) in vec_and_index()) {
             streaming_one_chunk_impl(seed, &data[offset..])?;
         }
     }
@@ -112,37 +112,37 @@ mod xxhash64 {
 
     proptest! {
         #[test]
-        fn oneshot_same_as_one_chunk(seed: u64, data: Vec<u8>) {
+        fn oneshot_same_as_one_chunk(seed in seed_64(), data: Vec<u8>) {
             oneshot_same_as_one_chunk_impl(seed, &data)?;
         }
 
         #[test]
-        fn oneshot_same_as_one_chunk_with_an_offset(seed: u64, (data, offset) in vec_and_index()) {
+        fn oneshot_same_as_one_chunk_with_an_offset(seed in seed_64(), (data, offset) in vec_and_index()) {
             oneshot_same_as_one_chunk_impl(seed, &data[offset..])?;
         }
 
         #[test]
-        fn oneshot_same_as_many_chunks(seed: u64, (data, chunks) in data_and_chunks()) {
+        fn oneshot_same_as_many_chunks(seed in seed_64(), (data, chunks) in data_and_chunks()) {
             oneshot_same_as_many_chunks_impl(seed, &data, &chunks)?;
         }
 
         #[test]
-        fn oneshot(seed: u64, data: Vec<u8>) {
+        fn oneshot(seed in seed_64(), data: Vec<u8>) {
             oneshot_impl(seed, &data)?;
         }
 
         #[test]
-        fn oneshot_with_an_offset(seed: u64, (data, offset) in vec_and_index()) {
+        fn oneshot_with_an_offset(seed in seed_64(), (data, offset) in vec_and_index()) {
             oneshot_impl(seed, &data[offset..])?;
         }
 
         #[test]
-        fn streaming_one_chunk(seed: u64, data: Vec<u8>) {
+        fn streaming_one_chunk(seed in seed_64(), data: Vec<u8>) {
             streaming_one_chunk_impl(seed, &data)?;
         }
 
         #[test]
-        fn streaming_one_chunk_with_an_offset(seed: u64, (data, offset) in vec_and_index()) {
+        fn streaming_one_chunk_with_an_offset(seed in seed_64(), (data, offset) in vec_and_index()) {
             streaming_one_chunk_impl(seed, &data[offset..])?;
         }
     }
@@ -212,27 +212,27 @@ mod xxhash3_64 {
 
     proptest! {
         #[test]
-        fn oneshot_same_as_one_chunk(seed: u64, data: Vec<u8>) {
+        fn oneshot_same_as_one_chunk(seed in seed_64(), data: Vec<u8>) {
             oneshot_same_as_one_chunk_impl(seed, &data)?;
         }
 
         #[test]
-        fn oneshot_same_as_one_chunk_with_an_offset(seed: u64, (data, offset) in vec_and_index()) {
+        fn oneshot_same_as_one_chunk_with_an_offset(seed in seed_64(), (data, offset) in vec_and_index()) {
             oneshot_same_as_one_chunk_impl(seed, &data[offset..])?;
         }
 
         #[test]
-        fn oneshot_same_as_many_chunks(seed: u64, (data, chunks) in data_and_chunks()) {
+        fn oneshot_same_as_many_chunks(seed in seed_64(), (data, chunks) in data_and_chunks()) {
             oneshot_same_as_many_chunks_impl(seed, &data, &chunks)?;
         }
 
         #[test]
-        fn oneshot(seed: u64, data: Vec<u8>) {
+        fn oneshot(seed in seed_64(), data: Vec<u8>) {
             oneshot_impl(seed, &data)?;
         }
 
         #[test]
-        fn oneshot_with_an_offset(seed: u64, (data, offset) in vec_and_index()) {
+        fn oneshot_with_an_offset(seed in seed_64(), (data, offset) in vec_and_index()) {
             oneshot_impl(seed, &data[offset..])?;
         }
 
@@ -242,22 +242,22 @@ mod xxhash3_64 {
         }
 
         #[test]
-        fn oneshot_with_a_seed_and_secret(seed: u64, secret in secret(), data: Vec<u8>) {
+        fn oneshot_with_a_seed_and_secret(seed in seed_64(), secret in secret(), data: Vec<u8>) {
             oneshot_with_seed_and_secret_impl(seed, &secret, &data)?;
         }
 
         #[test]
-        fn streaming_one_chunk(seed: u64, data: Vec<u8>) {
+        fn streaming_one_chunk(seed in seed_64(), data: Vec<u8>) {
             streaming_one_chunk_impl(seed, &data)?;
         }
 
         #[test]
-        fn streaming_one_chunk_with_an_offset(seed: u64, (data, offset) in vec_and_index()) {
+        fn streaming_one_chunk_with_an_offset(seed in seed_64(), (data, offset) in vec_and_index()) {
             streaming_one_chunk_impl(seed, &data[offset..])?;
         }
 
         #[test]
-        fn streaming_with_a_seed_and_secret(seed: u64, secret in secret(), data: Vec<u8>) {
+        fn streaming_with_a_seed_and_secret(seed in seed_64(), secret in secret(), data: Vec<u8>) {
             streaming_with_seed_and_secret_impl(seed, &secret, &data)?;
         }
     }
@@ -363,6 +363,14 @@ mod xxhash3_64 {
     }
 }
 
+fn seed_32() -> impl Strategy<Value = u32> {
+    prop_oneof![Just(0), Just(u32::MAX), num::u32::ANY]
+}
+
+fn seed_64() -> impl Strategy<Value = u64> {
+    prop_oneof![Just(0), Just(u64::MAX), num::u64::ANY]
+}
+
 fn vec_and_index() -> impl Strategy<Value = (Vec<u8>, usize)> {
     prop::collection::vec(num::u8::ANY, 0..=32 * 1024).prop_flat_map(|vec| {
         let len = vec.len();

diff --git a/src/xxhash3_64.rs b/src/xxhash3_64.rs
@@ -896,7 +896,10 @@ fn impl_1_to_3_bytes(secret: &Secret, seed: u64, input: &[u8]) -> u64 {
 
     let secret_words = secret.words_for_1_to_3();
 
-    let value = ((secret_words[0] ^ secret_words[1]).into_u64() + seed) ^ combined.into_u64();
+    let value = {
+        let secret = (secret_words[0] ^ secret_words[1]).into_u64();
+        secret.wrapping_add(seed) ^ combined.into_u64()
+    };
 
     // FUTURE: TEST: "Note that the XXH3-64 result is the lower half of XXH3-128 result."
     avalanche_xxh64(value)