hemantk-12 commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1401426786
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketHead.java:
##########
@@ -66,8 +65,8 @@ public void testHeadFail() throws Exception {
try {
Review Comment:
nit: please change this to `assertThrow()`.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestEndpointBase.java:
##########
@@ -96,10 +99,10 @@ public void init() { }
try {
Review Comment:
nit:
```suggestion
OS3Exception ex = assertThrows(OS3Exception.class,
() -> endpointBase.getCustomMetadataFromHeaders(s3requestHeaders));
assertTrue(ex.getCode().contains("MetadataTooLarge"));
```
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java:
##########
@@ -201,8 +175,8 @@ public void testDeleteKeys() throws IOException,
OS3Exception {
MultiDeleteResponse response =
bucketEndpoint.multiDelete("BucketName", "keyName", request);
- Assert.assertTrue(response.getErrors().size() == 1);
- Assert.assertTrue(
+ assertTrue(response.getErrors().size() == 1);
Review Comment:
nit: use assertEqual().
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestListParts.java:
##########
@@ -130,7 +131,7 @@ public void testListPartsWithUnknownUploadID() throws
Exception {
REST.get(OzoneConsts.S3_BUCKET, OzoneConsts.KEY, 0,
uploadID, 2, "0");
} catch (OS3Exception ex) {
- Assert.assertEquals(S3ErrorTable.NO_SUCH_UPLOAD.getErrorMessage(),
+ assertEquals(S3ErrorTable.NO_SUCH_UPLOAD.getErrorMessage(),
Review Comment:
Same comment as in TestBucketPut.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java:
##########
@@ -65,8 +65,8 @@ public void testBucketFailWithAuthHeaderMissing() throws
Exception {
try {
bucketEndpoint.put(bucketName, null, null, null);
} catch (OS3Exception ex) {
- Assert.assertEquals(HTTP_NOT_FOUND, ex.getHttpCode());
- Assert.assertEquals(MALFORMED_HEADER.getCode(), ex.getCode());
+ assertEquals(HTTP_NOT_FOUND, ex.getHttpCode());
Review Comment:
This test is not doing anything if I'm not wrong because it will never reach
to catch. If I'm right, please create a jira to fix it. Same for
`testBucketFailWithInvalidHeader`.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java:
##########
@@ -198,8 +198,8 @@ public void testCreateBucketFailure() throws Exception {
fail();
} catch (OS3Exception ex) {
Review Comment:
nit: please use assertThrow() here and in others tests of this class. If
this ask is dragging migration jira, feel free to create improvement jira to
use `assertThrow()` instead of try-catch block.
--
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]
