hemantk-12 commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1399620897
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java:
##########
@@ -248,15 +248,15 @@ public void testCopyObject() throws IOException,
OS3Exception {
keyContent = IOUtils.toString(ozoneInputStream, UTF_8);
- Assert.assertEquals(200, response.getStatus());
- Assert.assertEquals(CONTENT, keyContent);
+ assertEquals(200, response.getStatus());
+ assertEquals(CONTENT, keyContent);
// source and dest same
try {
objectEndpoint.put(bucketName, keyName, CONTENT.length(), 1, null, body);
fail("test copy object failed");
} catch (OS3Exception ex) {
- Assert.assertTrue(ex.getErrorMessage().contains("This copy request is " +
+ assertTrue(ex.getErrorMessage().contains("This copy request is " +
Review Comment:
nit: please use assertThrow() here and other tests in this class.
##########
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:
nit: please use assertThrow().
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketAcl.java:
##########
@@ -259,7 +262,7 @@ public void testBucketNotExist() throws Exception {
try {
bucketEndpoint.getAcl("bucket-not-exist");
Review Comment:
nit: please use assertThrow().
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectHead.java:
##########
@@ -98,11 +98,11 @@ public void testHeadFailByBadName() throws Exception {
//Head an object that doesn't exist.
try {
Response response = keyEndpoint.head(bucketName, "badKeyName");
- Assert.assertEquals(404, response.getStatus());
+ assertEquals(404, response.getStatus());
} catch (OS3Exception ex) {
- Assert.assertTrue(ex.getCode().contains("NoSuchObject"));
- Assert.assertTrue(ex.getErrorMessage().contains("object does not
exist"));
- Assert.assertEquals(HTTP_NOT_FOUND, ex.getHttpCode());
+ assertTrue(ex.getCode().contains("NoSuchObject"));
Review Comment:
nit: please use assertThrow().
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java:
##########
@@ -96,10 +98,10 @@ public void testListS3Buckets() throws IOException {
try {
rootEndpoint.get();
- Assert.fail("Should fail");
+ fail("Should fail");
} catch (Exception e) {
- Assert.assertTrue(e instanceof OS3Exception);
- Assert.assertTrue(((OS3Exception) e).getHttpCode() == HTTP_FORBIDDEN);
+ assertTrue(e instanceof OS3Exception);
Review Comment:
nit: please use assertThrow() for whole test class.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketList.java:
##########
@@ -353,8 +357,8 @@ public void listWithContinuationTokenFail() throws
IOException {
null, null, null).getEntity();
fail("listWithContinuationTokenFail");
} catch (OS3Exception ex) {
- Assert.assertEquals("random", ex.getResource());
- Assert.assertEquals("Invalid Argument", ex.getErrorMessage());
+ assertEquals("random", ex.getResource());
Review Comment:
nit: please use assertThrow().
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectGet.java:
##########
@@ -177,33 +177,33 @@ public void getRangeHeader() throws IOException,
OS3Exception {
Response response;
Mockito.when(headers.getHeaderString(RANGE_HEADER)).thenReturn("bytes=0-0");
- response = rest.get("b1", "key1", 0, null, 0, null);
- Assert.assertEquals("1", response.getHeaderString("Content-Length"));
- Assert.assertEquals(String.format("bytes 0-0/%s", CONTENT.length()),
+ response = rest.get("b1", "key1", null, 0, null);
Review Comment:
Curious if it is valid change. I pull the change and saw build failure. Also
it is same in master branch
https://github.com/apache/ozone/blob/master/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectGet.java#L180
--
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]