suztomo commented on a change in pull request #14527:
URL: https://github.com/apache/beam/pull/14527#discussion_r615109285
##########
File path:
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImplTest.java
##########
@@ -89,28 +91,35 @@
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
/** Tests for {@link BigQueryServicesImpl}. */
@RunWith(JUnit4.class)
public class BigQueryServicesImplTest {
@Rule public ExpectedException thrown = ExpectedException.none();
@Rule public ExpectedLogs expectedLogs =
ExpectedLogs.none(BigQueryServicesImpl.class);
- @Mock private LowLevelHttpResponse response;
+ // A test can make mock responses through setupMockResponses
+ private LowLevelHttpResponse[] responses;
Review comment:
Previously one mock response object was reused throughout multiple HTTP
responses. This does not work any more as newer google-http-client calls
`getStatusCode()` multiple times per response.
Now this test prepares different mock response objects for different
responses.
##########
File path:
sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilTest.java
##########
@@ -497,21 +497,30 @@ public void testGetSizeBytesWhenFileNotFoundBatchRetry()
throws Exception {
+ "\n";
thrown.expect(FileNotFoundException.class);
- final LowLevelHttpResponse mockResponse =
Mockito.mock(LowLevelHttpResponse.class);
- when(mockResponse.getContentType()).thenReturn("multipart/mixed;
boundary=" + contentBoundary);
+ final LowLevelHttpResponse[] mockResponses =
+ new LowLevelHttpResponse[] {
+ Mockito.mock(LowLevelHttpResponse.class),
Mockito.mock(LowLevelHttpResponse.class),
Review comment:
2 mock objects for 2 responses.
##########
File path:
sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/util/RetryHttpRequestInitializerTest.java
##########
@@ -209,23 +231,30 @@ public void testThrowIOException() throws IOException {
verify(mockLowLevelRequest, times(2)).setTimeout(anyInt(), anyInt());
verify(mockLowLevelRequest, times(2)).setWriteTimeout(anyInt());
verify(mockLowLevelRequest, times(2)).execute();
- verify(mockLowLevelResponse).getStatusCode();
+ verify(mockLowLevelResponse, atLeastOnce()).getStatusCode();
expectedLogs.verifyDebug("Request failed with IOException");
}
/** Tests that a retryable error is retried enough times. */
@Test
public void testRetryableErrorRetryEnoughTimes() throws IOException {
- when(mockLowLevelRequest.execute()).thenReturn(mockLowLevelResponse);
+ List<MockLowLevelHttpResponse> responses = new ArrayList<>();
final int retries = 10;
- when(mockLowLevelResponse.getStatusCode())
+
+ // The underlying http library calls getStatusCode method of a response
multiple times. For a
+ // response, the method should return the same value. Therefore this test
cannot rely on
+ // `mockLowLevelResponse` variable that are reused across responses.
+ when(mockLowLevelRequest.execute())
.thenAnswer(
- new Answer<Integer>() {
+ new Answer<MockLowLevelHttpResponse>() {
int n = 0;
@Override
- public Integer answer(InvocationOnMock invocation) {
- return n++ < retries ? 503 : 9999;
+ public MockLowLevelHttpResponse answer(InvocationOnMock
invocation) throws Throwable {
+ MockLowLevelHttpResponse response =
mock(MockLowLevelHttpResponse.class);
+ responses.add(response);
+ when(response.getStatusCode()).thenReturn(n++ < retries ? 503
: 9999);
+ return response;
Review comment:
Previously it was reusing the same mock object for different responses.
Now it prepares different mock objects for different responses.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
