Migrate CodedOutputStreamTest to @Parameterized.

These tests were (generally) looping over OutputType already. Some tests were looping over a subset; I've expanded many tests to loop over more OutputTypes.

But the first failure they encountered with any OutputType meant they'd halt
that test, without testing the other OutputTypes. That's frustrating.

We use `assume()` to discard tests in the matrix that are irrelevant.

There are many java parameterized test runners.  I followed the lead of
third_party/java_src/protobuf/current/javatests/com/google/protobuf/IsValidUtf8FourByteTest.java,
which uses Paramaterized runner.

This means:
- We see which output type is failing in the test name.
- We don't have to always assertWithMessage(OutputType.name()). We can just use
  assertThat. Nice.
- It's really easy to add new coders, and run all the tests against them. I've
  done that here for NIO encoders with offset, increasing their test coverage.

PiperOrigin-RevId: 677564209
pull/18441/head
Mark Hansen 5 months ago committed by Copybara-Service
parent ecf5f2e047
commit 5553ff1e50
  1. 255
      java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java

@ -9,6 +9,8 @@ package com.google.protobuf;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.common.truth.TruthJUnit.assume;
import static org.junit.Assert.assertThrows;
import com.google.protobuf.CodedOutputStream.OutOfSpaceException;
import protobuf_unittest.UnittestProto.SparseEnumMessage;
@ -18,19 +20,30 @@ import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
/** Unit test for {@link CodedOutputStream}. */
@RunWith(JUnit4.class)
@RunWith(Parameterized.class)
public class CodedOutputStreamTest {
@Parameters(name = "OutputType={0}")
public static List<OutputType> data() {
return Arrays.asList(OutputType.values());
}
private final OutputType outputType;
public CodedOutputStreamTest(OutputType outputType) {
this.outputType = outputType;
}
private interface Coder {
CodedOutputStream stream();
byte[] toByteArray();
OutputType getOutputType();
}
private static final class OutputStreamCoder implements Coder {
@ -51,11 +64,6 @@ public class CodedOutputStreamTest {
public byte[] toByteArray() {
return output.toByteArray();
}
@Override
public OutputType getOutputType() {
return OutputType.STREAM;
}
}
private static final class ArrayCoder implements Coder {
@ -76,11 +84,6 @@ public class CodedOutputStreamTest {
public byte[] toByteArray() {
return Arrays.copyOf(bytes, stream.getTotalBytesWritten());
}
@Override
public OutputType getOutputType() {
return OutputType.ARRAY;
}
}
private static final class NioHeapCoder implements Coder {
@ -114,25 +117,18 @@ public class CodedOutputStreamTest {
dup.get(bytes);
return bytes;
}
@Override
public OutputType getOutputType() {
return OutputType.NIO_HEAP;
}
}
private static final class NioDirectCoder implements Coder {
private final int initialPosition;
private final CodedOutputStream stream;
private final ByteBuffer buffer;
private final boolean unsafe;
NioDirectCoder(int size, boolean unsafe) {
this(size, 0, unsafe);
}
NioDirectCoder(int size, int initialPosition, boolean unsafe) {
this.unsafe = unsafe;
this.initialPosition = initialPosition;
buffer = ByteBuffer.allocateDirect(size);
buffer.position(initialPosition);
@ -157,11 +153,6 @@ public class CodedOutputStreamTest {
dup.get(bytes);
return bytes;
}
@Override
public OutputType getOutputType() {
return unsafe ? OutputType.NIO_DIRECT_SAFE : OutputType.NIO_DIRECT_UNSAFE;
}
}
private enum OutputType {
@ -177,16 +168,37 @@ public class CodedOutputStreamTest {
return new NioHeapCoder(size);
}
},
NIO_HEAP_WITH_INITIAL_OFFSET() {
@Override
Coder newCoder(int size) {
int offset = 2;
return new NioHeapCoder(size + offset, /* initialPosition= */ offset);
}
},
NIO_DIRECT_SAFE() {
@Override
Coder newCoder(int size) {
return new NioDirectCoder(size, false);
return new NioDirectCoder(size, /* unsafe= */ false);
}
},
NIO_DIRECT_SAFE_WITH_INITIAL_OFFSET() {
@Override
Coder newCoder(int size) {
int offset = 2;
return new NioDirectCoder(size + offset, offset, /* unsafe= */ false);
}
},
NIO_DIRECT_UNSAFE() {
@Override
Coder newCoder(int size) {
return new NioDirectCoder(size, true);
return new NioDirectCoder(size, /* unsafe= */ true);
}
},
NIO_DIRECT_UNSAFE_WITH_INITIAL_OFFSET() {
@Override
Coder newCoder(int size) {
int offset = 2;
return new NioDirectCoder(size + offset, offset, /* unsafe= */ true);
}
},
STREAM() {
@ -202,15 +214,13 @@ public class CodedOutputStreamTest {
/** Checks that invariants are maintained for varint round trip input and output. */
@Test
public void testVarintRoundTrips() throws Exception {
for (OutputType outputType : OutputType.values()) {
assertVarintRoundTrip(outputType, 0L);
assertVarintRoundTrip(0L);
for (int bits = 0; bits < 64; bits++) {
long value = 1L << bits;
assertVarintRoundTrip(outputType, value);
assertVarintRoundTrip(outputType, value + 1);
assertVarintRoundTrip(outputType, value - 1);
assertVarintRoundTrip(outputType, -value);
}
assertVarintRoundTrip(value);
assertVarintRoundTrip(value + 1);
assertVarintRoundTrip(value - 1);
assertVarintRoundTrip(-value);
}
}
@ -273,6 +283,10 @@ public class CodedOutputStreamTest {
/** Test encodeZigZag32() and encodeZigZag64(). */
@Test
public void testEncodeZigZag() throws Exception {
// We only need to run this test once, they don't depend on outputType.
// Arbitrarily run them just for ARRAY.
assume().that(outputType).isEqualTo(OutputType.ARRAY);
assertThat(CodedOutputStream.encodeZigZag32(0)).isEqualTo(0);
assertThat(CodedOutputStream.encodeZigZag32(-1)).isEqualTo(1);
assertThat(CodedOutputStream.encodeZigZag32(1)).isEqualTo(2);
@ -326,6 +340,10 @@ public class CodedOutputStreamTest {
@Test
public void computeIntSize() {
// We only need to run this test once, they don't depend on outputType.
// Arbitrarily run them just for ARRAY.
assume().that(outputType).isEqualTo(OutputType.ARRAY);
assertThat(CodedOutputStream.computeUInt32SizeNoTag(0)).isEqualTo(1);
assertThat(CodedOutputStream.computeUInt64SizeNoTag(0)).isEqualTo(1);
int i;
@ -378,6 +396,10 @@ public class CodedOutputStreamTest {
@Test
public void computeTagSize() {
// We only need to run this test once, they don't depend on outputType.
// Arbitrarily run them just for ARRAY.
assume().that(outputType).isEqualTo(OutputType.ARRAY);
assertThat(CodedOutputStream.computeTagSize(0)).isEqualTo(1);
int i;
for (i = 0; i < 4; i++) {
@ -408,7 +430,6 @@ public class CodedOutputStreamTest {
SparseEnumMessage message =
SparseEnumMessage.newBuilder().setSparseEnum(TestSparseEnum.SPARSE_E).build();
assertThat(message.getSparseEnum().getNumber()).isLessThan(0);
for (OutputType outputType : OutputType.values()) {
Coder coder = outputType.newCoder(message.getSerializedSize());
message.writeTo(coder.stream());
coder.stream().flush();
@ -416,12 +437,13 @@ public class CodedOutputStreamTest {
SparseEnumMessage message2 = SparseEnumMessage.parseFrom(rawBytes);
assertThat(message2.getSparseEnum()).isEqualTo(TestSparseEnum.SPARSE_E);
}
}
/** Test getTotalBytesWritten() */
@Test
public void testGetTotalBytesWritten() throws Exception {
Coder coder = OutputType.STREAM.newCoder(4 * 1024);
assume().that(outputType).isEqualTo(OutputType.STREAM);
Coder coder = outputType.newCoder(4 * 1024);
// Write some some bytes (more than the buffer can hold) and verify that totalWritten
// is correct.
@ -463,15 +485,15 @@ public class CodedOutputStreamTest {
.isEqualTo(2);
assertThat(bufferSize).isEqualTo(string.length() * Utf8.MAX_BYTES_PER_CHAR);
for (OutputType outputType : OutputType.values()) {
Coder coder = outputType.newCoder(bufferSize + 2);
coder.stream().writeStringNoTag(string);
coder.stream().flush();
}
}
@Test
public void testWriteToByteBuffer() throws Exception {
assume().that(outputType).isEqualTo(OutputType.NIO_HEAP);
final int bufferSize = 16 * 1024;
ByteBuffer buffer = ByteBuffer.allocate(bufferSize);
CodedOutputStream codedStream = CodedOutputStream.newInstance(buffer);
@ -507,8 +529,8 @@ public class CodedOutputStreamTest {
@Test
public void testWriteByteBuffer() throws Exception {
byte[] value = "abcde".getBytes(Internal.UTF_8);
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
CodedOutputStream codedStream = CodedOutputStream.newInstance(outputStream);
Coder coder = outputType.newCoder(100);
CodedOutputStream codedStream = coder.stream();
ByteBuffer byteBuffer = ByteBuffer.wrap(value, 0, 1);
// This will actually write 5 bytes into the CodedOutputStream as the
// ByteBuffer's capacity() is 5.
@ -521,7 +543,7 @@ public class CodedOutputStreamTest {
codedStream.writeRawBytes(ByteBuffer.wrap(value, 2, 1).slice());
codedStream.flush();
byte[] result = outputStream.toByteArray();
byte[] result = coder.toByteArray();
assertThat(result).hasLength(6);
for (int i = 0; i < 5; i++) {
assertThat(value[i]).isEqualTo(result[i]);
@ -531,14 +553,14 @@ public class CodedOutputStreamTest {
@Test
public void testWriteByteArrayWithOffsets() throws Exception {
assume().that(outputType).isEqualTo(OutputType.ARRAY);
byte[] fullArray = bytes(0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88);
for (OutputType type : new OutputType[] {OutputType.ARRAY}) {
Coder coder = type.newCoder(4);
Coder coder = outputType.newCoder(4);
coder.stream().writeByteArrayNoTag(fullArray, 2, 2);
assertWithMessage(type.name()).that(coder.toByteArray()).isEqualTo(bytes(0x02, 0x33, 0x44));
assertThat(coder.toByteArray()).isEqualTo(bytes(0x02, 0x33, 0x44));
assertThat(coder.stream().getTotalBytesWritten()).isEqualTo(3);
}
}
@Test
public void testSerializeUtf8_multipleSmallWrites() throws Exception {
@ -553,16 +575,14 @@ public class CodedOutputStreamTest {
}
final byte[] expectedBytes = expectedBytesStream.toByteArray();
// For each output type, write the source string 2 bytes at a time and verify the output.
for (OutputType outputType : OutputType.values()) {
// Write the source string 2 bytes at a time and verify the output.
Coder coder = outputType.newCoder(expectedBytes.length);
for (int pos = 0; pos < source.length(); pos += 2) {
String substr = source.substring(pos, pos + 2);
coder.stream().writeStringNoTag(substr);
}
coder.stream().flush();
assertWithMessage(outputType.name()).that(coder.toByteArray()).isEqualTo(expectedBytes);
}
assertThat(coder.toByteArray()).isEqualTo(expectedBytes);
}
@Test
@ -576,16 +596,11 @@ public class CodedOutputStreamTest {
newString(Character.MIN_HIGH_SURROGATE, Character.MIN_HIGH_SURROGATE)
};
CodedOutputStream outputWithStream = CodedOutputStream.newInstance(new ByteArrayOutputStream());
CodedOutputStream outputWithArray = CodedOutputStream.newInstance(new byte[10000]);
CodedOutputStream outputWithByteBuffer =
CodedOutputStream.newInstance(ByteBuffer.allocate(10000));
Coder coder = outputType.newCoder(10000);
for (String s : invalidStrings) {
// TODO: These should all fail; instead they are corrupting data.
CodedOutputStream.computeStringSizeNoTag(s);
outputWithStream.writeStringNoTag(s);
outputWithArray.writeStringNoTag(s);
outputWithByteBuffer.writeStringNoTag(s);
coder.stream().writeStringNoTag(s);
}
}
@ -593,20 +608,16 @@ public class CodedOutputStreamTest {
// encoding invalid UTF-8 strings.
@Test
public void testSerializeInvalidUtf8FollowedByOutOfSpace() throws Exception {
// Streaming's buffering masks out of space errors.
assume().that(outputType).isNotEqualTo(OutputType.STREAM);
final int notEnoughBytes = 4;
CodedOutputStream outputWithArray = CodedOutputStream.newInstance(new byte[notEnoughBytes]);
CodedOutputStream outputWithByteBuffer =
CodedOutputStream.newInstance(ByteBuffer.allocate(notEnoughBytes));
Coder coder = outputType.newCoder(notEnoughBytes);
String invalidString = newString(Character.MIN_HIGH_SURROGATE, 'f', 'o', 'o', 'b', 'a', 'r');
try {
outputWithArray.writeStringNoTag(invalidString);
assertWithMessage("Expected OutOfSpaceException").fail();
} catch (OutOfSpaceException e) {
assertThat(e).hasCauseThat().isInstanceOf(IndexOutOfBoundsException.class);
}
try {
outputWithByteBuffer.writeStringNoTag(invalidString);
coder.stream().writeStringNoTag(invalidString);
assertWithMessage("Expected OutOfSpaceException").fail();
} catch (OutOfSpaceException e) {
assertThat(e).hasCauseThat().isInstanceOf(IndexOutOfBoundsException.class);
@ -620,23 +631,16 @@ public class CodedOutputStreamTest {
assertThat(CodedOutputStream.computeUInt32SizeNoTag(testCase.length()))
.isEqualTo(CodedOutputStream.computeUInt32SizeNoTag(testCase.length() * 3));
assertThat(CodedOutputStream.computeStringSize(1, testCase)).isEqualTo(11);
// Tag is one byte, varint describing string length is 1 byte, string length is 9 bytes.
// An array of size 1 will cause a failure when trying to write the varint.
for (OutputType outputType :
new OutputType[] {
OutputType.ARRAY,
OutputType.NIO_HEAP,
OutputType.NIO_DIRECT_SAFE,
OutputType.NIO_DIRECT_UNSAFE
}) {
// Stream's buffering means we don't throw.
assume().that(outputType).isNotEqualTo(OutputType.STREAM);
for (int i = 0; i < 11; i++) {
Coder coder = outputType.newCoder(i);
try {
coder.stream().writeString(1, testCase);
assertWithMessage("Should have thrown an out of space exception").fail();
} catch (CodedOutputStream.OutOfSpaceException expected) {
}
}
assertThrows(OutOfSpaceException.class, () -> coder.stream().writeString(1, testCase));
}
}
@ -657,49 +661,41 @@ public class CodedOutputStreamTest {
(1 << 17) - 1,
// 3 bytes for ASCII and Unicode
};
for (OutputType outputType : OutputType.values()) {
for (int i : lengths) {
testEncodingOfString(outputType, 'q', i); // 1 byte per char
testEncodingOfString(outputType, '\u07FF', i); // 2 bytes per char
testEncodingOfString(outputType, '\u0981', i); // 3 bytes per char
}
testEncodingOfString('q', i); // 1 byte per char
testEncodingOfString('\u07FF', i); // 2 bytes per char
testEncodingOfString('\u0981', i); // 3 bytes per char
}
}
@Test
public void testNioEncodersWithInitialOffsets() throws Exception {
String value = "abc";
for (Coder coder :
new Coder[] {
new NioHeapCoder(10, 2), new NioDirectCoder(10, 2, false), new NioDirectCoder(10, 2, true)
}) {
coder.stream().writeStringNoTag(value);
public void testWriteSmallString() throws Exception {
Coder coder = outputType.newCoder(10);
coder.stream().writeStringNoTag("abc");
coder.stream().flush();
assertWithMessage(coder.getOutputType().name())
.that(coder.toByteArray())
.isEqualTo(new byte[] {3, 'a', 'b', 'c'});
}
assertThat(coder.toByteArray()).isEqualTo(new byte[] {3, 'a', 'b', 'c'});
}
/**
* Parses the given bytes using writeRawLittleEndian32() and checks that the result matches the
* given value.
*/
private static void assertWriteLittleEndian32(byte[] data, int value) throws Exception {
for (OutputType outputType : OutputType.values()) {
private void assertWriteLittleEndian32(byte[] data, int value) throws Exception {
{
Coder coder = outputType.newCoder(data.length);
coder.stream().writeFixed32NoTag(value);
coder.stream().flush();
assertWithMessage(outputType.name()).that(coder.toByteArray()).isEqualTo(data);
assertThat(coder.toByteArray()).isEqualTo(data);
}
// Try different block sizes.
// If streaming, try different block sizes.
if (outputType == OutputType.STREAM) {
for (int blockSize = 1; blockSize <= 16; blockSize *= 2) {
OutputType outputType = OutputType.STREAM;
Coder coder = outputType.newCoder(blockSize);
coder.stream().writeFixed32NoTag(value);
coder.stream().flush();
assertWithMessage(outputType.name()).that(coder.toByteArray()).isEqualTo(data);
assertThat(coder.toByteArray()).isEqualTo(data);
}
}
}
@ -707,21 +703,22 @@ public class CodedOutputStreamTest {
* Parses the given bytes using writeRawLittleEndian64() and checks that the result matches the
* given value.
*/
private static void assertWriteLittleEndian64(byte[] data, long value) throws Exception {
for (OutputType outputType : OutputType.values()) {
private void assertWriteLittleEndian64(byte[] data, long value) throws Exception {
{
Coder coder = outputType.newCoder(data.length);
coder.stream().writeFixed64NoTag(value);
coder.stream().flush();
assertWithMessage(outputType.name()).that(coder.toByteArray()).isEqualTo(data);
assertThat(coder.toByteArray()).isEqualTo(data);
}
// Try different block sizes.
// If streaming, try different block sizes.
if (outputType == OutputType.STREAM) {
for (int blockSize = 1; blockSize <= 16; blockSize *= 2) {
OutputType outputType = OutputType.STREAM;
Coder coder = outputType.newCoder(blockSize);
coder.stream().writeFixed64NoTag(value);
coder.stream().flush();
assertWithMessage(outputType.name()).that(coder.toByteArray()).isEqualTo(data);
assertThat(coder.toByteArray()).isEqualTo(data);
}
}
}
@ -729,15 +726,13 @@ public class CodedOutputStreamTest {
return new String(chars);
}
private static void testEncodingOfString(OutputType outputType, char c, int length)
throws Exception {
private void testEncodingOfString(char c, int length) throws Exception {
String fullString = fullString(c, length);
TestAllTypes testAllTypes = TestAllTypes.newBuilder().setOptionalString(fullString).build();
Coder coder = outputType.newCoder(testAllTypes.getSerializedSize());
testAllTypes.writeTo(coder.stream());
coder.stream().flush();
assertWithMessage("OutputType: " + outputType)
.that(fullString)
assertThat(fullString)
.isEqualTo(TestAllTypes.parseFrom(coder.toByteArray()).getOptionalString());
}
@ -764,14 +759,13 @@ public class CodedOutputStreamTest {
* result matches the given bytes.
*/
@SuppressWarnings("UnnecessaryLongToIntConversion") // Intentionally tests 32-bit int values.
private static void assertWriteVarint(byte[] data, long value) throws Exception {
for (OutputType outputType : OutputType.values()) {
private void assertWriteVarint(byte[] data, long value) throws Exception {
// Only test 32-bit write if the value fits into an int.
if (value == (int) value) {
Coder coder = outputType.newCoder(10);
coder.stream().writeUInt32NoTag((int) value);
coder.stream().flush();
assertWithMessage(outputType.name()).that(coder.toByteArray()).isEqualTo(data);
assertThat(coder.toByteArray()).isEqualTo(data);
// Also try computing size.
assertThat(data).hasLength(CodedOutputStream.computeUInt32SizeNoTag((int) value));
@ -781,51 +775,48 @@ public class CodedOutputStreamTest {
Coder coder = outputType.newCoder(10);
coder.stream().writeUInt64NoTag(value);
coder.stream().flush();
assertWithMessage(outputType.name()).that(coder.toByteArray()).isEqualTo(data);
assertThat(coder.toByteArray()).isEqualTo(data);
// Also try computing size.
assertThat(data).hasLength(CodedOutputStream.computeUInt64SizeNoTag(value));
}
}
// Try different block sizes.
// If streaming, try different block sizes.
if (outputType == OutputType.STREAM) {
for (int blockSize = 1; blockSize <= 16; blockSize *= 2) {
// Only test 32-bit write if the value fits into an int.
if (value == (int) value) {
OutputType outputType = OutputType.STREAM;
Coder coder = outputType.newCoder(blockSize);
coder.stream().writeUInt64NoTag((int) value);
coder.stream().flush();
assertWithMessage(outputType.name()).that(coder.toByteArray()).isEqualTo(data);
assertThat(coder.toByteArray()).isEqualTo(data);
ByteArrayOutputStream rawOutput = new ByteArrayOutputStream();
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput, blockSize);
output.writeUInt32NoTag((int) value);
output.flush();
assertWithMessage(outputType.name()).that(rawOutput.toByteArray()).isEqualTo(data);
assertThat(rawOutput.toByteArray()).isEqualTo(data);
}
{
OutputType outputType = OutputType.STREAM;
Coder coder = outputType.newCoder(blockSize);
coder.stream().writeUInt64NoTag(value);
coder.stream().flush();
assertWithMessage(outputType.name()).that(coder.toByteArray()).isEqualTo(data);
assertThat(coder.toByteArray()).isEqualTo(data);
}
}
}
}
private static void assertVarintRoundTrip(OutputType outputType, long value) throws Exception {
private void assertVarintRoundTrip(long value) throws Exception {
{
Coder coder = outputType.newCoder(10);
coder.stream().writeUInt64NoTag(value);
coder.stream().flush();
byte[] bytes = coder.toByteArray();
assertWithMessage(outputType.name())
.that(bytes)
.hasLength(CodedOutputStream.computeUInt64SizeNoTag(value));
assertThat(bytes).hasLength(CodedOutputStream.computeUInt64SizeNoTag(value));
CodedInputStream input = CodedInputStream.newInstance(new ByteArrayInputStream(bytes));
assertWithMessage(outputType.name()).that(input.readRawVarint64()).isEqualTo(value);
assertThat(input.readRawVarint64()).isEqualTo(value);
}
if (value == (int) value) {
@ -833,11 +824,9 @@ public class CodedOutputStreamTest {
coder.stream().writeUInt32NoTag((int) value);
coder.stream().flush();
byte[] bytes = coder.toByteArray();
assertWithMessage(outputType.name())
.that(bytes)
.hasLength(CodedOutputStream.computeUInt32SizeNoTag((int) value));
assertThat(bytes).hasLength(CodedOutputStream.computeUInt32SizeNoTag((int) value));
CodedInputStream input = CodedInputStream.newInstance(new ByteArrayInputStream(bytes));
assertWithMessage(outputType.name()).that(input.readRawVarint32()).isEqualTo(value);
assertThat(input.readRawVarint32()).isEqualTo(value);
}
}
}

Loading…
Cancel
Save