You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "raju-balpande (via GitHub)" <gi...@apache.org> on 2023/11/16 17:46:19 UTC
[PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
raju-balpande opened a new pull request, #5616:
URL: https://github.com/apache/ozone/pull/5616
## What changes were proposed in this pull request?
Migration of Junit tests in ozone-s3gateway to JUnit5
Please describe your PR in detail:
Migration of Junit tests in ozone-s3gateway to JUnit5 for the following files:
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestAbortMultipartUpload.java
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketAcl.java
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketDelete.java
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketHead.java
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketList.java
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketResponse.java
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9499
## How was this patch tested?
All of the test cases are checked manually before adding it in commits.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5616:
URL: https://github.com/apache/ozone/pull/5616#issuecomment-1823211928
Thanks @raju-balpande for the patch, @Galsza, @hemantk-12 for the review.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1401839806
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java:
##########
@@ -93,14 +95,8 @@ public void testListS3Buckets() throws IOException {
doThrow(exception).when(objectStore).getS3Volume();
RootEndpoint rootEndpoint = new RootEndpoint();
rootEndpoint.setClient(client);
-
- try {
- rootEndpoint.get();
- Assert.fail("Should fail");
- } catch (Exception e) {
- Assert.assertTrue(e instanceof OS3Exception);
- Assert.assertTrue(((OS3Exception) e).getHttpCode() == HTTP_FORBIDDEN);
- }
+ OS3Exception e = assertThrows(OS3Exception.class, () -> rootEndpoint.get());
+ assertEquals(e.getHttpCode(), HTTP_FORBIDDEN);
Review Comment:
Done, Thanks,
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5616:
URL: https://github.com/apache/ozone/pull/5616#issuecomment-1821319709
Thanks @raju-balpande for updating the patch.
There are a few JUnit4 assertions left in s3gateway:
```
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
59:import static org.junit.Assert.assertEquals;
60:import static org.junit.Assert.assertTrue;
61:import static org.junit.Assert.fail;
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestAuthorizationFilter.java
50:import static org.junit.Assert.fail;
53:import org.junit.Assert;
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/exception/TestOS3Exceptions.java
21:import static org.junit.Assert.assertEquals;
```
If the PR replaced these, too, then you could also add the `<allow.junit4>false` property to ban the old version.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1400364067
##########
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:
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1400364723
##########
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:
Done.
##########
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:
Done.
##########
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:
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1402452670
##########
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:
[HDDS-9760](https://issues.apache.org/jira/browse/HDDS-9760)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 merged PR #5616:
URL: https://github.com/apache/ozone/pull/5616
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1400081257
##########
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:
I noticed this, there is change in parameters in the called method in parallel.
Done. Thanks
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1401418890
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java:
##########
@@ -93,14 +95,8 @@ public void testListS3Buckets() throws IOException {
doThrow(exception).when(objectStore).getS3Volume();
RootEndpoint rootEndpoint = new RootEndpoint();
rootEndpoint.setClient(client);
-
- try {
- rootEndpoint.get();
- Assert.fail("Should fail");
- } catch (Exception e) {
- Assert.assertTrue(e instanceof OS3Exception);
- Assert.assertTrue(((OS3Exception) e).getHttpCode() == HTTP_FORBIDDEN);
- }
+ OS3Exception e = assertThrows(OS3Exception.class, () -> rootEndpoint.get());
+ assertEquals(e.getHttpCode(), HTTP_FORBIDDEN);
Review Comment:
nit: expected is supposed to be on left side and actual on the right.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5616:
URL: https://github.com/apache/ozone/pull/5616#issuecomment-1822480841
Thanks @raju-balpande for updating the patch. There are some checkstyle failures:
```
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketHead.java
66: Line is longer than 80 characters (found 98).
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
160: Line is longer than 80 characters (found 97).
210: Line is longer than 80 characters (found 95).
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1401844078
##########
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:
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1399637060
##########
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:
This is failing in your fork's workflow as well: https://github.com/raju-balpande/apache_ozone/actions/runs/6928699242/job/18844989213
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on PR #5616:
URL: https://github.com/apache/ozone/pull/5616#issuecomment-1821289730
I converted the '@Test(expected=XXX)' to assertThrows and all the applicable try-catch to assertThrows.
In some case I see the exception handling is done but the catch block is not part of expected flow and hence in such cases it is kept as it is.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1401843664
##########
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:
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on PR #5616:
URL: https://github.com/apache/ozone/pull/5616#issuecomment-1823006015
@raju-balpande thank you for the patch this is looking good to me. (Minor advice that you can do to make reviewing your changes easier is to describe what your commit does in the commit message instead of repeating the same message for all commits)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1401840210
##########
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:
Done, thanks.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1401843064
##########
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:
Created [HDDS-9760](https://issues.apache.org/jira/browse/HDDS-9760) to address the logical error in three test cases where the catch block is not getting executed.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1400081257
##########
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:
I noticed this, there is change in parameters in the called method in parallel. Will address this. thanks.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1400364528
##########
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:
Done. Thanks.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9499. Migrate tests in ozone-s3gateway to JUnit5 [ozone]
Posted by "raju-balpande (via GitHub)" <gi...@apache.org>.
raju-balpande commented on code in PR #5616:
URL: https://github.com/apache/ozone/pull/5616#discussion_r1400363724
##########
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:
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org