From 9f84559fa82101763acefe04a39cfda379ee33c7 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Mon, 13 Jan 2025 07:20:50 -0800 Subject: [PATCH] Make `UnsignedBytesTest` actually check that we are using the correct endianness for reads. I noticed this during work to [avoid using `Unsafe`](https://github.com/google/guava/issues/6806) (though it applies even if we do use `Unsafe`, as you'd guess from the `BIG_ENDIAN` check there). RELNOTES=n/a PiperOrigin-RevId: 714958103 --- .../common/primitives/UnsignedBytesTest.java | 57 ++++++++++++------- .../common/primitives/UnsignedBytesTest.java | 57 ++++++++++++------- 2 files changed, 74 insertions(+), 40 deletions(-) diff --git a/android/guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java b/android/guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java index da00a4274088..e2c5ee57a60e 100644 --- a/android/guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java +++ b/android/guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java @@ -20,6 +20,7 @@ import static com.google.common.primitives.UnsignedBytes.min; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static java.lang.Math.signum; import static org.junit.Assert.assertThrows; import com.google.common.collect.testing.Helpers; @@ -95,8 +96,8 @@ public void testCompare() { byte y = VALUES[j]; // note: spec requires only that the sign is the same assertWithMessage(x + ", " + y) - .that(Math.signum(UnsignedBytes.compare(x, y))) - .isEqualTo(Math.signum(Integer.compare(i, j))); + .that(signum(UnsignedBytes.compare(x, y))) + .isEqualTo(signum(Integer.compare(i, j))); } } } @@ -264,7 +265,7 @@ public void testLexicographicalComparator() { new byte[] {GREATEST, GREATEST}, new byte[] {GREATEST, GREATEST, GREATEST}); - // The Unsafe implementation if it's available. Otherwise, the Java implementation. + // The VarHandle, Unsafe, or Java implementation. Comparator comparator = UnsignedBytes.lexicographicalComparator(); Helpers.testComparator(comparator, ordered); assertThat(SerializableTester.reserialize(comparator)).isSameInstanceAs(comparator); @@ -275,26 +276,42 @@ public void testLexicographicalComparator() { assertThat(SerializableTester.reserialize(javaImpl)).isSameInstanceAs(javaImpl); } - public void testLexicographicalComparatorLongInputs() { - Random rnd = new Random(); - for (Comparator comparator : - Arrays.asList( - UnsignedBytes.lexicographicalComparator(), - UnsignedBytes.lexicographicalComparatorJavaImpl())) { - for (int trials = 10; trials-- > 0; ) { - byte[] left = new byte[1 + rnd.nextInt(32)]; - rnd.nextBytes(left); - byte[] right = left.clone(); - assertThat(comparator.compare(left, right)).isEqualTo(0); - int i = rnd.nextInt(left.length); - left[i] ^= (byte) (1 + rnd.nextInt(255)); - assertThat(comparator.compare(left, right)).isNotEqualTo(0); - assertThat(UnsignedBytes.compare(left[i], right[i]) > 0) - .isEqualTo(comparator.compare(left, right) > 0); - } + public void testLexicographicalComparatorLongPseudorandomInputs() { + Comparator comparator1 = UnsignedBytes.lexicographicalComparator(); + Comparator comparator2 = UnsignedBytes.lexicographicalComparatorJavaImpl(); + Random rnd = new Random(714958103); + for (int trial = 0; trial < 100; trial++) { + byte[] left = new byte[1 + rnd.nextInt(32)]; + rnd.nextBytes(left); + byte[] right = left.clone(); + assertThat(comparator1.compare(left, right)).isEqualTo(0); + assertThat(comparator2.compare(left, right)).isEqualTo(0); + int i = rnd.nextInt(left.length); + left[i] ^= (byte) (1 + rnd.nextInt(255)); + assertThat(signum(comparator1.compare(left, right))) + .isEqualTo(signum(UnsignedBytes.compare(left[i], right[i]))); + assertThat(signum(comparator2.compare(left, right))) + .isEqualTo(signum(UnsignedBytes.compare(left[i], right[i]))); } } + public void testLexicographicalComparatorLongHandwrittenInputs() { + Comparator comparator1 = UnsignedBytes.lexicographicalComparator(); + Comparator comparator2 = UnsignedBytes.lexicographicalComparatorJavaImpl(); + + /* + * These arrays are set up to test that the comparator compares bytes within a word in the + * correct order—in order words, that it doesn't mix up big-endian and little-endian. The first + * array has a smaller element at one index, and then the second array has a smaller elements at + * the next. + */ + byte[] a0 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 99, 15, 16, 17}; + byte[] b0 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 99, 14, 15, 16, 17}; + + assertThat(comparator1.compare(a0, b0)).isLessThan(0); + assertThat(comparator2.compare(a0, b0)).isLessThan(0); + } + public void testSort() { testSort(new byte[] {}, new byte[] {}); testSort(new byte[] {2}, new byte[] {2}); diff --git a/guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java b/guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java index 85f8e98439b4..908abe57b3ac 100644 --- a/guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java +++ b/guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java @@ -21,6 +21,7 @@ import static com.google.common.primitives.UnsignedBytes.min; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static java.lang.Math.signum; import static org.junit.Assert.assertThrows; import com.google.common.collect.testing.Helpers; @@ -96,8 +97,8 @@ public void testCompare() { byte y = VALUES[j]; // note: spec requires only that the sign is the same assertWithMessage(x + ", " + y) - .that(Math.signum(UnsignedBytes.compare(x, y))) - .isEqualTo(Math.signum(Integer.compare(i, j))); + .that(signum(UnsignedBytes.compare(x, y))) + .isEqualTo(signum(Integer.compare(i, j))); } } } @@ -267,7 +268,7 @@ public void testLexicographicalComparator() { new byte[] {GREATEST, GREATEST}, new byte[] {GREATEST, GREATEST, GREATEST}); - // The Unsafe implementation if it's available. Otherwise, the Java implementation. + // The VarHandle, Unsafe, or Java implementation. Comparator comparator = UnsignedBytes.lexicographicalComparator(); Helpers.testComparator(comparator, ordered); assertThat(SerializableTester.reserialize(comparator)).isSameInstanceAs(comparator); @@ -278,26 +279,42 @@ public void testLexicographicalComparator() { assertThat(SerializableTester.reserialize(javaImpl)).isSameInstanceAs(javaImpl); } - public void testLexicographicalComparatorLongInputs() { - Random rnd = new Random(); - for (Comparator comparator : - Arrays.asList( - UnsignedBytes.lexicographicalComparator(), - UnsignedBytes.lexicographicalComparatorJavaImpl())) { - for (int trials = 10; trials-- > 0; ) { - byte[] left = new byte[1 + rnd.nextInt(32)]; - rnd.nextBytes(left); - byte[] right = left.clone(); - assertThat(comparator.compare(left, right)).isEqualTo(0); - int i = rnd.nextInt(left.length); - left[i] ^= (byte) (1 + rnd.nextInt(255)); - assertThat(comparator.compare(left, right)).isNotEqualTo(0); - assertThat(UnsignedBytes.compare(left[i], right[i]) > 0) - .isEqualTo(comparator.compare(left, right) > 0); - } + public void testLexicographicalComparatorLongPseudorandomInputs() { + Comparator comparator1 = UnsignedBytes.lexicographicalComparator(); + Comparator comparator2 = UnsignedBytes.lexicographicalComparatorJavaImpl(); + Random rnd = new Random(714958103); + for (int trial = 0; trial < 100; trial++) { + byte[] left = new byte[1 + rnd.nextInt(32)]; + rnd.nextBytes(left); + byte[] right = left.clone(); + assertThat(comparator1.compare(left, right)).isEqualTo(0); + assertThat(comparator2.compare(left, right)).isEqualTo(0); + int i = rnd.nextInt(left.length); + left[i] ^= (byte) (1 + rnd.nextInt(255)); + assertThat(signum(comparator1.compare(left, right))) + .isEqualTo(signum(UnsignedBytes.compare(left[i], right[i]))); + assertThat(signum(comparator2.compare(left, right))) + .isEqualTo(signum(UnsignedBytes.compare(left[i], right[i]))); } } + public void testLexicographicalComparatorLongHandwrittenInputs() { + Comparator comparator1 = UnsignedBytes.lexicographicalComparator(); + Comparator comparator2 = UnsignedBytes.lexicographicalComparatorJavaImpl(); + + /* + * These arrays are set up to test that the comparator compares bytes within a word in the + * correct order—in order words, that it doesn't mix up big-endian and little-endian. The first + * array has a smaller element at one index, and then the second array has a smaller elements at + * the next. + */ + byte[] a0 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 99, 15, 16, 17}; + byte[] b0 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 99, 14, 15, 16, 17}; + + assertThat(comparator1.compare(a0, b0)).isLessThan(0); + assertThat(comparator2.compare(a0, b0)).isLessThan(0); + } + public void testSort() { testSort(new byte[] {}, new byte[] {}); testSort(new byte[] {2}, new byte[] {2});