Skip to content

Improve test clarity and execution time. #1743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 2, 2025
Merged

Improve test clarity and execution time. #1743

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7080d43
Improve test clarity and performance.
vbabanin Jun 17, 2025
16e12f5
Adjust code point filtering in tests.
vbabanin Jun 17, 2025
3aa864e
Refactor ByteBufferBsonInputTest to enhance test clarity.
vbabanin Jun 17, 2025
ade2870
Fix checkstyle.
vbabanin Jun 18, 2025
b28e02b
Change parameter and test names.
vbabanin Jul 2, 2025
5624f0a
Merge branch 'main' into JAVA-5898
vbabanin Jul 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 77 additions & 46 deletions driver-core/src/test/unit/com/mongodb/internal/connection/ByteBufferBsonInputTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,37 +54,68 @@ class ByteBufferBsonInputTest {
private static final List<Integer> ALL_CODE_POINTS_EXCLUDING_SURROGATES = Stream.concat(
range(1, MIN_HIGH_SURROGATE).boxed(),
rangeClosed(MAX_LOW_SURROGATE + 1, MAX_CODE_POINT).boxed())
.filter(i -> i < 128 || i % 10 == 0) // only subset of code points to speed up testing
.filter(i -> i < 128 || i % 30 == 0) // only subset of code points to speed up testing
.collect(toList());

static Stream<BufferProvider> bufferProviders() {
return Stream.of(
size -> new NettyByteBuf(PooledByteBufAllocator.DEFAULT.directBuffer(size)),
size -> new NettyByteBuf(PooledByteBufAllocator.DEFAULT.heapBuffer(size)),
new PowerOfTwoBufferPool(),
size -> new ByteBufNIO(ByteBuffer.wrap(new byte[size + 5], 2, size).slice()), //different array offsets
size -> new ByteBufNIO(ByteBuffer.wrap(new byte[size + 4], 3, size).slice()), //different array offsets
size -> new ByteBufNIO(ByteBuffer.allocateDirect(size)),
size -> new ByteBufNIO(ByteBuffer.allocate(size)) {
@Override
public boolean isBackedByArray() {
return false;
}

@Override
public byte[] array() {
return Assertions.fail("array() is called, when isBackedByArray() returns false");
}

@Override
public int arrayOffset() {
return Assertions.fail("arrayOffset() is called, when isBackedByArray() returns false");
}
}
createBufferProvider(
"NettyByteBuf based on PooledByteBufAllocator.DEFAULT.directBuffer",
size -> new NettyByteBuf(PooledByteBufAllocator.DEFAULT.directBuffer(size))
),
createBufferProvider(
"NettyByteBuf based on PooledByteBufAllocator.DEFAULT.heapBuffer",
size -> new NettyByteBuf(PooledByteBufAllocator.DEFAULT.heapBuffer(size))
),
createBufferProvider(
"PowerOfTwoBufferPool",
new PowerOfTwoBufferPool()
),
createBufferProvider(
"ByteBufNIO based on ByteBuffer with arrayOffset() -> 2",
size -> new ByteBufNIO(ByteBuffer.wrap(new byte[size + 5], 2, size).slice())
),
createBufferProvider(
"ByteBufNIO based on ByteBuffer with arrayOffset() -> 3,",
size -> new ByteBufNIO(ByteBuffer.wrap(new byte[size + 4], 3, size).slice())
),
createBufferProvider(
"ByteBufNIO emulating direct ByteBuffer",
size -> new ByteBufNIO(ByteBuffer.allocate(size)) {
@Override
public boolean isBackedByArray() {
return false;
}

@Override
public byte[] array() {
return Assertions.fail("array() is called, when isBackedByArray() returns false");
}

@Override
public int arrayOffset() {
return Assertions.fail("arrayOffset() is called, when isBackedByArray() returns false");
}
}
)
);
}

@ParameterizedTest
private static BufferProvider createBufferProvider(final String bufferName, final BufferProvider bufferProvider) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about my point. Is it true that bufferDescription is more appropriate naming? Saw it is used to overwrite toString.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, changed to bufferDescription.

return new BufferProvider() {
@Override
public ByteBuf getBuffer(final int size) {
return bufferProvider.getBuffer(size);
}

@Override
public String toString() {
return bufferName;
}
};
}

@ParameterizedTest(name = "should read empty string. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadEmptyString(final BufferProvider bufferProvider) {
// given
Expand All @@ -101,7 +132,7 @@ void shouldReadEmptyString(final BufferProvider bufferProvider) {
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read empty CString. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadEmptyCString(final BufferProvider bufferProvider) {
// given
Expand All @@ -116,7 +147,7 @@ void shouldReadEmptyCString(final BufferProvider bufferProvider) {
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read invalid one byte string. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadInvalidOneByteString(final BufferProvider bufferProvider) {
ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{2, 0, 0, 0, (byte) 0xFF, 0});
Expand All @@ -131,7 +162,7 @@ void shouldReadInvalidOneByteString(final BufferProvider bufferProvider) {
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read invalid one byte CString. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadInvalidOneByteCString(final BufferProvider bufferProvider) {
ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, new byte[]{-0x01, 0});
Expand All @@ -147,7 +178,7 @@ void shouldReadInvalidOneByteCString(final BufferProvider bufferProvider) {
}


@ParameterizedTest
@ParameterizedTest(name = "should read string up to buffer limit. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadStringUptoBufferLimit(final BufferProvider bufferProvider) {
// given
Expand All @@ -171,7 +202,7 @@ void shouldReadStringUptoBufferLimit(final BufferProvider bufferProvider) {
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read string with more data in buffer. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadStringWithMoreDataInBuffer(final BufferProvider bufferProvider) throws IOException {
// given
Expand Down Expand Up @@ -200,7 +231,7 @@ void shouldReadStringWithMoreDataInBuffer(final BufferProvider bufferProvider) t
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read multiple strings within buffer. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadMultipleStringsWithinBuffer(final BufferProvider bufferProvider) throws IOException {
// given
Expand Down Expand Up @@ -252,7 +283,7 @@ void shouldReadMultipleStringsWithinBuffer(final BufferProvider bufferProvider)
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read consecutive multiple strings within buffer. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadConsecutiveMultipleStringsWithinBuffer(final BufferProvider bufferProvider) throws IOException {
// given
Expand Down Expand Up @@ -302,7 +333,7 @@ void shouldReadConsecutiveMultipleStringsWithinBuffer(final BufferProvider buffe
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read consecutive multiple CStrings within buffer. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadConsecutiveMultipleCStringsWithinBuffer(final BufferProvider bufferProvider) throws IOException {
// given
Expand Down Expand Up @@ -352,7 +383,7 @@ void shouldReadConsecutiveMultipleCStringsWithinBuffer(final BufferProvider buff
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read multiple CStrings within buffer. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadMultipleCStringsWithinBuffer(final BufferProvider bufferProvider) throws IOException {
// given
Expand Down Expand Up @@ -409,7 +440,7 @@ void shouldReadMultipleCStringsWithinBuffer(final BufferProvider bufferProvider)
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read string within buffer. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadStringWithinBuffer(final BufferProvider bufferProvider) throws IOException {
// given
Expand Down Expand Up @@ -441,7 +472,7 @@ void shouldReadStringWithinBuffer(final BufferProvider bufferProvider) throws IO
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read CString up to buffer limit. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadCStringUptoBufferLimit(final BufferProvider bufferProvider) {
// given
Expand All @@ -465,7 +496,7 @@ void shouldReadCStringUptoBufferLimit(final BufferProvider bufferProvider) {
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read CString with more data in buffer. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadCStringWithMoreDataInBuffer(final BufferProvider bufferProvider) throws IOException {
// given
Expand Down Expand Up @@ -494,7 +525,7 @@ void shouldReadCStringWithMoreDataInBuffer(final BufferProvider bufferProvider)
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read CString within buffer. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldReadCStringWithingBuffer(final BufferProvider bufferProvider) throws IOException {
// given
Expand Down Expand Up @@ -526,7 +557,7 @@ void shouldReadCStringWithingBuffer(final BufferProvider bufferProvider) throws
}
}

@ParameterizedTest
@ParameterizedTest(name = "should throw if CString is not null terminated skip. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldThrowIfCStringIsNotNullTerminatedSkip(final BufferProvider bufferProvider) {
// given
Expand All @@ -553,7 +584,7 @@ public static Stream<Arguments> nonNullTerminatedStringsWithBuffers() {
return arguments.stream();
}

@ParameterizedTest
@ParameterizedTest(name = "should throw if string is not null terminated. Parameters: nonNullTerminatedString={0}, bufferProvider={1}")
@MethodSource("nonNullTerminatedStringsWithBuffers")
void shouldThrowIfStringIsNotNullTerminated(final byte[] nonNullTerminatedString, final BufferProvider bufferProvider) {
// given
Expand All @@ -579,7 +610,7 @@ public static Stream<Arguments> nonNullTerminatedCStringsWithBuffers() {
return arguments.stream();
}

@ParameterizedTest
@ParameterizedTest(name = "should throw if CString is not null terminated. Parameters: nonNullTerminatedCString={0}, bufferProvider={1}")
@MethodSource("nonNullTerminatedCStringsWithBuffers")
void shouldThrowIfCStringIsNotNullTerminated(final byte[] nonNullTerminatedCString, final BufferProvider bufferProvider) {
// given
Expand All @@ -592,7 +623,7 @@ void shouldThrowIfCStringIsNotNullTerminated(final byte[] nonNullTerminatedCStri
}


@ParameterizedTest
@ParameterizedTest(name = "should throw if one byte string is not null terminated. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldThrowIfOneByteStringIsNotNullTerminated(final BufferProvider bufferProvider) {
// given
Expand All @@ -604,7 +635,7 @@ void shouldThrowIfOneByteStringIsNotNullTerminated(final BufferProvider bufferPr
}
}

@ParameterizedTest
@ParameterizedTest(name = "should throw if one byte CString is not null terminated. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldThrowIfOneByteCStringIsNotNullTerminated(final BufferProvider bufferProvider) {
// given
Expand All @@ -616,7 +647,7 @@ void shouldThrowIfOneByteCStringIsNotNullTerminated(final BufferProvider bufferP
}
}

@ParameterizedTest
@ParameterizedTest(name = "should throw if length of bson string is not positive. BufferProvider={0}")
@MethodSource("bufferProviders")
void shouldThrowIfLengthOfBsonStringIsNotPositive(final BufferProvider bufferProvider) {
// given
Expand Down Expand Up @@ -644,8 +675,8 @@ public static Stream<Arguments> shouldSkipCStringWhenMultipleNullTerminationPres
return arguments.stream();
}

@ParameterizedTest
@MethodSource()
@ParameterizedTest(name = "should skip CString when multiple null termination present. Parameters: cStringBytes={0}, bufferProvider={1}")
@MethodSource
void shouldSkipCStringWhenMultipleNullTerminationPresent(final byte[] cStringBytes, final BufferProvider bufferProvider) {
// given
ByteBuf buffer = allocateAndWriteToBuffer(bufferProvider, cStringBytes);
Expand All @@ -660,7 +691,7 @@ void shouldSkipCStringWhenMultipleNullTerminationPresent(final byte[] cStringByt
}
}

@ParameterizedTest
@ParameterizedTest(name = "should read skip CString when multiple null termination present within buffer. BufferProvider={0}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "multiple null terminations" preferable?

Copy link
Member Author

@vbabanin vbabanin Jul 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it has to be plural. And now I think the correct term here is “terminators,” not “termination,” so I changed it to “terminators.”

@MethodSource("bufferProviders")
void shouldReadSkipCStringWhenMultipleNullTerminationPresentWithinBuffer(final BufferProvider bufferProvider) {
// given
Expand Down
Loading