hemantk-12 commented on code in PR #4573:
URL: https://github.com/apache/ozone/pull/4573#discussion_r1191702155
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -4123,4 +4124,34 @@ private OzoneBucket getBucket(OzoneVolume volume) throws
IOException {
private static ReplicationConfig anyReplication() {
return
RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.ONE);
}
+
+ private void assertBucketHasSnapshotCount(OzoneVolume volume,
Review Comment:
nit: alignment is bit off.
```suggestion
private void assertBucketHasSnapshotCount(OzoneVolume volume,
String bucketPrefix,
String preBucket,
boolean hasSnapshot,
int expectedBucketCount) {
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -4123,4 +4124,34 @@ private OzoneBucket getBucket(OzoneVolume volume) throws
IOException {
private static ReplicationConfig anyReplication() {
return
RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.ONE);
}
+
Review Comment:
assertBucketHasSnapshotCount and assertBucketCount can be a simple one
function.
1.
```
private void assertBucketCount(OzoneVolume volume,
String bucketPrefix,
boolean hasSnapshot,
int expectedBucketCount) {
Iterator<? extends OzoneBucket> bucketIterator;
if (hasSnapshot) {
bucketIterator = volume.listBuckets(bucketPrefix, null, hasSnapshot);
} else {
bucketIterator = volume.listBuckets(bucketPrefix);
}
int bucketCount = 0;
while (bucketIterator.hasNext()) {
Assert.assertTrue(
bucketIterator.next().getName().startsWith(bucketPrefix));
bucketCount++;
}
Assert.assertEquals(expectedBucketCount, bucketCount);
}
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -2247,69 +2247,70 @@ public void testListBucket()
OzoneVolume volA = store.getVolume(volumeA);
OzoneVolume volB = store.getVolume(volumeB);
+
//Create 10 buckets in vol-a-<random> and 10 in vol-b-<random>
String bucketBaseNameA = "bucket-a-";
for (int i = 0; i < 10; i++) {
- volA.createBucket(
- bucketBaseNameA + i + "-" + RandomStringUtils.randomNumeric(5));
- volB.createBucket(
- bucketBaseNameA + i + "-" + RandomStringUtils.randomNumeric(5));
+ String bucketName = bucketBaseNameA +
+ i + "-" + RandomStringUtils.randomNumeric(5);
+ volA.createBucket(bucketName);
+ store.createSnapshot(volumeA, bucketName, null);
+ bucketName = bucketBaseNameA +
+ i + "-" + RandomStringUtils.randomNumeric(5);
+ volB.createBucket(bucketName);
+ store.createSnapshot(volumeB, bucketName, null);
}
//Create 10 buckets in vol-a-<random> and 10 in vol-b-<random>
String bucketBaseNameB = "bucket-b-";
for (int i = 0; i < 10; i++) {
- volA.createBucket(
- bucketBaseNameB + i + "-" + RandomStringUtils.randomNumeric(5));
+ String bucketName = bucketBaseNameB +
+ i + "-" + RandomStringUtils.randomNumeric(5);
+ volA.createBucket(bucketName);
+ store.createSnapshot(volumeA, bucketName, null);
volB.createBucket(
bucketBaseNameB + i + "-" + RandomStringUtils.randomNumeric(5));
}
- Iterator<? extends OzoneBucket> volABucketIter =
- volA.listBuckets("bucket-");
- int volABucketCount = 0;
- while (volABucketIter.hasNext()) {
- volABucketIter.next();
- volABucketCount++;
- }
- Assert.assertEquals(20, volABucketCount);
- Iterator<? extends OzoneBucket> volBBucketIter =
- volA.listBuckets("bucket-");
- int volBBucketCount = 0;
- while (volBBucketIter.hasNext()) {
- volBBucketIter.next();
- volBBucketCount++;
- }
- Assert.assertEquals(20, volBBucketCount);
+ assertBucketCount(volA, "bucket-", 20);
+ assertBucketHasSnapshotCount(volA, "bucket-", null, true, 20);
+ assertBucketCount(volB, "bucket-", 20);
+ assertBucketHasSnapshotCount(volB, "bucket-", null, true, 10);
+ assertBucketCount(volA, bucketBaseNameA, 10);
+ assertBucketHasSnapshotCount(volA, bucketBaseNameA, null, true, 10);
+ assertBucketCount(volB, bucketBaseNameB, 10);
+ assertBucketHasSnapshotCount(volB, bucketBaseNameB, null, true, 0);
- Iterator<? extends OzoneBucket> volABucketAIter =
- volA.listBuckets("bucket-a-");
- int volABucketACount = 0;
- while (volABucketAIter.hasNext()) {
- volABucketAIter.next();
- volABucketACount++;
- }
- Assert.assertEquals(10, volABucketACount);
- Iterator<? extends OzoneBucket> volBBucketBIter =
- volA.listBuckets("bucket-b-");
- int volBBucketBCount = 0;
- while (volBBucketBIter.hasNext()) {
- volBBucketBIter.next();
- volBBucketBCount++;
- }
- Assert.assertEquals(10, volBBucketBCount);
Iterator<? extends OzoneBucket> volABucketBIter = volA.listBuckets(
- "bucket-b-");
+ bucketBaseNameB);
for (int i = 0; i < 10; i++) {
Assert.assertTrue(volABucketBIter.next().getName()
.startsWith(bucketBaseNameB + i + "-"));
}
Assert.assertFalse(volABucketBIter.hasNext());
+
+ Iterator<? extends OzoneBucket> volABucketBHasSnapshotIter =
Review Comment:
nit: this is also repetitive code and can be extracted out to function as
done for `assertBucketCount`.
--
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]
