adoroszlai commented on code in PR #11397:
URL: https://github.com/apache/ozone/pull/11397#discussion_r4182462914


##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java:
##########
@@ -156,6 +180,91 @@ public void testListAllBucketsUnpaginatedReturnsAll() 
throws Exception {
     assertNull(response.getContinuationToken());
   }
 
+  @ParameterizedTest(name = "directory={0}, buckets={1}, limit={2}, 
previous={3}")
+  @CsvSource({
+      // directory, total buckets, response limit, previous index, expected 
names, continuation token, RPC calls
+      "false, 0, 2, -1, '', false, 1",
+      "false, 1, 2, -1, bucket-0, false, 2",

Review Comment:
   Maybe it's just me, but I find `@CsvSource` to be hard to read, and I guess 
it will be difficult to update (e.g. add a new param).



##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java:
##########
@@ -156,6 +180,91 @@ public void testListAllBucketsUnpaginatedReturnsAll() 
throws Exception {
     assertNull(response.getContinuationToken());
   }
 
+  @ParameterizedTest(name = "directory={0}, buckets={1}, limit={2}, 
previous={3}")
+  @CsvSource({
+      // directory, total buckets, response limit, previous index, expected 
names, continuation token, RPC calls
+      "false, 0, 2, -1, '', false, 1",
+      "false, 1, 2, -1, bucket-0, false, 2",
+      "false, 2, 2, -1, bucket-0;bucket-1, false, 2",
+      "false, 3, 2, -1, bucket-0;bucket-1, true, 2",
+      "false, 2, 1, -1, bucket-0, true, 1",
+      "false, 3, 5, -1, bucket-0;bucket-1;bucket-2, false, 3",
+      "false, 3, 2, 1, bucket-2, false, 2",
+      "false, 3, , -1, bucket-0;bucket-1;bucket-2, false, 3",
+      "true, 0, 2, -1, '', false, 1",
+      "true, 1, 2, -1, bucket-0, false, 2",
+      "true, 2, 2, -1, bucket-0, false, 2",
+      "true, 3, 2, -1, bucket-0;bucket-2, false, 3",
+      "true, 4, 2, -1, bucket-0;bucket-2, true, 2",
+      "true, 2, 1, -1, bucket-0, true, 1",
+      "true, 3, 5, -1, bucket-0;bucket-2, false, 3",
+      "true, 3, 0, -1, '', false, 1",
+      "true, 5, 2, 2, bucket-4, false, 2",
+      "true, 2, 2, 0, '', false, 2"
+  })
+  void testListBucketsRpcCount(boolean directory, int total, Integer limit, 
int previous,
+      String expectedNames, boolean hasToken, int expectedCalls) throws 
Exception {
+    OzoneConfiguration conf = new OzoneConfiguration();
+    conf.setInt(OzoneConfigKeys.OZONE_CLIENT_LIST_CACHE_SIZE, 2);
+    ClientProtocol proxy = mock(ClientProtocol.class);
+    List<OzoneBucket> buckets = new ArrayList<>();
+    for (int i = 0; i < total; i++) {
+      buckets.add(OzoneBucket.newBuilder(conf, 
proxy).setVolumeName(DEFAULT_VOLUME).setName("bucket-" + i)
+          .setBucketLayout(i % 2 == 0 ? BucketLayout.FILE_SYSTEM_OPTIMIZED : 
BucketLayout.OBJECT_STORE).build());
+    }
+    when(proxy.listBuckets(eq(DEFAULT_VOLUME), isNull(), 
nullable(String.class), eq(2), eq(false)))
+        .thenAnswer(invocation -> {
+          String marker = invocation.getArgument(2);
+          return buckets.stream()
+              .filter(bucket -> marker == null || 
bucket.getName().compareTo(marker) > 0)
+              .limit(2).collect(Collectors.toList());
+        });
+    OzoneVolume volume = OzoneVolume.newBuilder(conf, 
proxy).setName(DEFAULT_VOLUME).setOwner("root")
+        .setAcls(Collections.emptyList()).build();
+    ObjectStore store = mock(ObjectStore.class);
+    when(store.getS3Volume()).thenReturn(volume);
+    when(store.getClientProxy()).thenReturn(proxy);
+    OzoneClient client = mock(OzoneClient.class);
+    when(client.getObjectStore()).thenReturn(store);
+    when(client.getProxy()).thenReturn(proxy);
+    RootEndpoint endpoint = 
EndpointBuilder.newRootEndpointBuilder().setClient(client)

Review Comment:
   The new test case sets up its own endpoint, does not exercise the 
`rootEndpoint` created in `@BeforeEach`.  It would be nice to move it to a 
separate class in a follow-up, maybe when all similar tasks are done.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to