Skip to content

Commit 71d0692

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Make UnsignedBytesTest actually check that we are using the correct endianness for reads.
I noticed this during work to [avoid using `Unsafe`](#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: 715368993
1 parent ee63055 commit 71d0692

File tree

2 files changed

+74
-40
lines changed

2 files changed

+74
-40
lines changed

android/guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static com.google.common.primitives.UnsignedBytes.min;
2121
import static com.google.common.truth.Truth.assertThat;
2222
import static com.google.common.truth.Truth.assertWithMessage;
23+
import static java.lang.Math.signum;
2324
import static org.junit.Assert.assertThrows;
2425

2526
import com.google.common.collect.testing.Helpers;
@@ -95,8 +96,8 @@ public void testCompare() {
9596
byte y = VALUES[j];
9697
// note: spec requires only that the sign is the same
9798
assertWithMessage(x + ", " + y)
98-
.that(Math.signum(UnsignedBytes.compare(x, y)))
99-
.isEqualTo(Math.signum(Integer.compare(i, j)));
99+
.that(signum(UnsignedBytes.compare(x, y)))
100+
.isEqualTo(signum(Integer.compare(i, j)));
100101
}
101102
}
102103
}
@@ -264,7 +265,7 @@ public void testLexicographicalComparator() {
264265
new byte[] {GREATEST, GREATEST},
265266
new byte[] {GREATEST, GREATEST, GREATEST});
266267

267-
// The Unsafe implementation if it's available. Otherwise, the Java implementation.
268+
// The VarHandle, Unsafe, or Java implementation.
268269
Comparator<byte[]> comparator = UnsignedBytes.lexicographicalComparator();
269270
Helpers.testComparator(comparator, ordered);
270271
assertThat(SerializableTester.reserialize(comparator)).isSameInstanceAs(comparator);
@@ -275,26 +276,42 @@ public void testLexicographicalComparator() {
275276
assertThat(SerializableTester.reserialize(javaImpl)).isSameInstanceAs(javaImpl);
276277
}
277278

278-
public void testLexicographicalComparatorLongInputs() {
279-
Random rnd = new Random();
280-
for (Comparator<byte[]> comparator :
281-
Arrays.asList(
282-
UnsignedBytes.lexicographicalComparator(),
283-
UnsignedBytes.lexicographicalComparatorJavaImpl())) {
284-
for (int trials = 10; trials-- > 0; ) {
285-
byte[] left = new byte[1 + rnd.nextInt(32)];
286-
rnd.nextBytes(left);
287-
byte[] right = left.clone();
288-
assertThat(comparator.compare(left, right)).isEqualTo(0);
289-
int i = rnd.nextInt(left.length);
290-
left[i] ^= (byte) (1 + rnd.nextInt(255));
291-
assertThat(comparator.compare(left, right)).isNotEqualTo(0);
292-
assertThat(UnsignedBytes.compare(left[i], right[i]) > 0)
293-
.isEqualTo(comparator.compare(left, right) > 0);
294-
}
279+
public void testLexicographicalComparatorLongPseudorandomInputs() {
280+
Comparator<byte[]> comparator1 = UnsignedBytes.lexicographicalComparator();
281+
Comparator<byte[]> comparator2 = UnsignedBytes.lexicographicalComparatorJavaImpl();
282+
Random rnd = new Random(714958103);
283+
for (int trial = 0; trial < 100; trial++) {
284+
byte[] left = new byte[1 + rnd.nextInt(32)];
285+
rnd.nextBytes(left);
286+
byte[] right = left.clone();
287+
assertThat(comparator1.compare(left, right)).isEqualTo(0);
288+
assertThat(comparator2.compare(left, right)).isEqualTo(0);
289+
int i = rnd.nextInt(left.length);
290+
left[i] ^= (byte) (1 + rnd.nextInt(255));
291+
assertThat(signum(comparator1.compare(left, right)))
292+
.isEqualTo(signum(UnsignedBytes.compare(left[i], right[i])));
293+
assertThat(signum(comparator2.compare(left, right)))
294+
.isEqualTo(signum(UnsignedBytes.compare(left[i], right[i])));
295295
}
296296
}
297297

298+
public void testLexicographicalComparatorLongHandwrittenInputs() {
299+
Comparator<byte[]> comparator1 = UnsignedBytes.lexicographicalComparator();
300+
Comparator<byte[]> comparator2 = UnsignedBytes.lexicographicalComparatorJavaImpl();
301+
302+
/*
303+
* These arrays are set up to test that the comparator compares bytes within a word in the
304+
* correct order—in order words, that it doesn't mix up big-endian and little-endian. The first
305+
* array has a smaller element at one index, and then the second array has a smaller elements at
306+
* the next.
307+
*/
308+
byte[] a0 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 99, 15, 16, 17};
309+
byte[] b0 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 99, 14, 15, 16, 17};
310+
311+
assertThat(comparator1.compare(a0, b0)).isLessThan(0);
312+
assertThat(comparator2.compare(a0, b0)).isLessThan(0);
313+
}
314+
298315
public void testSort() {
299316
testSort(new byte[] {}, new byte[] {});
300317
testSort(new byte[] {2}, new byte[] {2});

guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static com.google.common.primitives.UnsignedBytes.min;
2222
import static com.google.common.truth.Truth.assertThat;
2323
import static com.google.common.truth.Truth.assertWithMessage;
24+
import static java.lang.Math.signum;
2425
import static org.junit.Assert.assertThrows;
2526

2627
import com.google.common.collect.testing.Helpers;
@@ -96,8 +97,8 @@ public void testCompare() {
9697
byte y = VALUES[j];
9798
// note: spec requires only that the sign is the same
9899
assertWithMessage(x + ", " + y)
99-
.that(Math.signum(UnsignedBytes.compare(x, y)))
100-
.isEqualTo(Math.signum(Integer.compare(i, j)));
100+
.that(signum(UnsignedBytes.compare(x, y)))
101+
.isEqualTo(signum(Integer.compare(i, j)));
101102
}
102103
}
103104
}
@@ -267,7 +268,7 @@ public void testLexicographicalComparator() {
267268
new byte[] {GREATEST, GREATEST},
268269
new byte[] {GREATEST, GREATEST, GREATEST});
269270

270-
// The Unsafe implementation if it's available. Otherwise, the Java implementation.
271+
// The VarHandle, Unsafe, or Java implementation.
271272
Comparator<byte[]> comparator = UnsignedBytes.lexicographicalComparator();
272273
Helpers.testComparator(comparator, ordered);
273274
assertThat(SerializableTester.reserialize(comparator)).isSameInstanceAs(comparator);
@@ -278,26 +279,42 @@ public void testLexicographicalComparator() {
278279
assertThat(SerializableTester.reserialize(javaImpl)).isSameInstanceAs(javaImpl);
279280
}
280281

281-
public void testLexicographicalComparatorLongInputs() {
282-
Random rnd = new Random();
283-
for (Comparator<byte[]> comparator :
284-
Arrays.asList(
285-
UnsignedBytes.lexicographicalComparator(),
286-
UnsignedBytes.lexicographicalComparatorJavaImpl())) {
287-
for (int trials = 10; trials-- > 0; ) {
288-
byte[] left = new byte[1 + rnd.nextInt(32)];
289-
rnd.nextBytes(left);
290-
byte[] right = left.clone();
291-
assertThat(comparator.compare(left, right)).isEqualTo(0);
292-
int i = rnd.nextInt(left.length);
293-
left[i] ^= (byte) (1 + rnd.nextInt(255));
294-
assertThat(comparator.compare(left, right)).isNotEqualTo(0);
295-
assertThat(UnsignedBytes.compare(left[i], right[i]) > 0)
296-
.isEqualTo(comparator.compare(left, right) > 0);
297-
}
282+
public void testLexicographicalComparatorLongPseudorandomInputs() {
283+
Comparator<byte[]> comparator1 = UnsignedBytes.lexicographicalComparator();
284+
Comparator<byte[]> comparator2 = UnsignedBytes.lexicographicalComparatorJavaImpl();
285+
Random rnd = new Random(714958103);
286+
for (int trial = 0; trial < 100; trial++) {
287+
byte[] left = new byte[1 + rnd.nextInt(32)];
288+
rnd.nextBytes(left);
289+
byte[] right = left.clone();
290+
assertThat(comparator1.compare(left, right)).isEqualTo(0);
291+
assertThat(comparator2.compare(left, right)).isEqualTo(0);
292+
int i = rnd.nextInt(left.length);
293+
left[i] ^= (byte) (1 + rnd.nextInt(255));
294+
assertThat(signum(comparator1.compare(left, right)))
295+
.isEqualTo(signum(UnsignedBytes.compare(left[i], right[i])));
296+
assertThat(signum(comparator2.compare(left, right)))
297+
.isEqualTo(signum(UnsignedBytes.compare(left[i], right[i])));
298298
}
299299
}
300300

301+
public void testLexicographicalComparatorLongHandwrittenInputs() {
302+
Comparator<byte[]> comparator1 = UnsignedBytes.lexicographicalComparator();
303+
Comparator<byte[]> comparator2 = UnsignedBytes.lexicographicalComparatorJavaImpl();
304+
305+
/*
306+
* These arrays are set up to test that the comparator compares bytes within a word in the
307+
* correct order—in order words, that it doesn't mix up big-endian and little-endian. The first
308+
* array has a smaller element at one index, and then the second array has a smaller elements at
309+
* the next.
310+
*/
311+
byte[] a0 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 99, 15, 16, 17};
312+
byte[] b0 = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 99, 14, 15, 16, 17};
313+
314+
assertThat(comparator1.compare(a0, b0)).isLessThan(0);
315+
assertThat(comparator2.compare(a0, b0)).isLessThan(0);
316+
}
317+
301318
public void testSort() {
302319
testSort(new byte[] {}, new byte[] {});
303320
testSort(new byte[] {2}, new byte[] {2});

0 commit comments

Comments
 (0)