From 2459bac3b3f7d7370fb69e9c55b1035d85b74209 Mon Sep 17 00:00:00 2001 From: Chris Povirk Date: Wed, 7 Sep 2022 10:32:54 -0400 Subject: [PATCH] Use real objects instead of mocks. My motivation for making this change is that [`ByteBuffer` is becoming `sealed`](https://download.java.net/java/early_access/loom/docs/api/java.base/java/nio/ByteBuffer.html) in new versions of Java. This makes it impossible for Mockito's _current_ default mockmaker to mock it. That said, Mockito will likely [switch its default mockmaker](https://github.com/mockito/mockito/issues/2589) to an alternative that _is_ able to mock `sealed` classes. However, there are downside to that, such as [slower performance](https://github.com/mockito/mockito/issues/2589#issuecomment-1192725206), so it's probably better to leave our options open by avoiding mocking at all. And in this case, it's equally easy to use real objects. As a bonus, I think that real objects makes the code a little easier to follow: Before, we created mocks that the code under test never interacted with in any way. (The code just passed them through to a delegate.) When I first read the tests, I was confused, since I assumed that the mock we were creating was the same mock that we then passed to `verify` at the end of the method. That turned out not to be the case. Given that, I figured I'd switch not only to a real `ByteBuffer` but also to a real `OutputStream`. --- .../ForwardingReadableBufferTest.java | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/core/src/test/java/io/grpc/internal/ForwardingReadableBufferTest.java b/core/src/test/java/io/grpc/internal/ForwardingReadableBufferTest.java index b778f25e5de..029a406b1f5 100644 --- a/core/src/test/java/io/grpc/internal/ForwardingReadableBufferTest.java +++ b/core/src/test/java/io/grpc/internal/ForwardingReadableBufferTest.java @@ -17,13 +17,12 @@ package io.grpc.internal; import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import io.grpc.ForwardingTestUtil; +import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.OutputStream; import java.lang.reflect.Method; import java.nio.ByteBuffer; import java.util.Collections; @@ -36,13 +35,10 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; -/** - * Tests for {@link ForwardingReadableBuffer}. - */ +/** Tests for {@link ForwardingReadableBuffer}. */ @RunWith(JUnit4.class) public class ForwardingReadableBufferTest { - @Rule - public final MockitoRule mocks = MockitoJUnit.rule(); + @Rule public final MockitoRule mocks = MockitoJUnit.rule(); @Mock private ReadableBuffer delegate; private ForwardingReadableBuffer buffer; @@ -55,10 +51,7 @@ public void setUp() { @Test public void allMethodsForwarded() throws Exception { ForwardingTestUtil.testMethodsForwarded( - ReadableBuffer.class, - delegate, - buffer, - Collections.emptyList()); + ReadableBuffer.class, delegate, buffer, Collections.emptyList()); } @Test @@ -99,7 +92,7 @@ public void readBytes() { @Test public void readBytes_overload1() { - ByteBuffer dest = mock(ByteBuffer.class); + ByteBuffer dest = ByteBuffer.allocate(0); buffer.readBytes(dest); verify(delegate).readBytes(dest); @@ -107,7 +100,7 @@ public void readBytes_overload1() { @Test public void readBytes_overload2() throws IOException { - OutputStream dest = mock(OutputStream.class); + OutputStream dest = new ByteArrayOutputStream(); buffer.readBytes(dest, 1); verify(delegate).readBytes(dest, 1);