smengcl commented on code in PR #4538:
URL: https://github.com/apache/ozone/pull/4538#discussion_r1203198948
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java:
##########
@@ -705,18 +705,9 @@ public void testGetSetRevokeS3Secret() throws Exception {
OmTransportFactory.create(conf, ugi, null),
RandomStringUtils.randomAscii(5));
- //Creates a secret since it does not exist
+ // Creates a secret since it does not exist
S3SecretValue attempt1 = omClient.getS3Secret(username);
- //Fetches the secret from db since it was created in previous step
- S3SecretValue attempt2 = omClient.getS3Secret(username);
-
- //secret fetched on both attempts must be same
- assertEquals(attempt1.getAwsSecret(), attempt2.getAwsSecret());
-
- //access key fetched on both attempts must be same
- assertEquals(attempt1.getAwsAccessKey(), attempt2.getAwsAccessKey());
-
Review Comment:
Rather than simply removing the second `getS3Secret()`, we should confirm it
now throws expected exception:
```
// A second getS3Secret on the same username should throw exception
LambdaTestUtils.intercept(OMException.class,
"expected error message",
() -> omClient.getS3Secret(username));
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java:
##########
@@ -705,18 +705,9 @@ public void testGetSetRevokeS3Secret() throws Exception {
OmTransportFactory.create(conf, ugi, null),
RandomStringUtils.randomAscii(5));
- //Creates a secret since it does not exist
+ // Creates a secret since it does not exist
S3SecretValue attempt1 = omClient.getS3Secret(username);
- //Fetches the secret from db since it was created in previous step
- S3SecretValue attempt2 = omClient.getS3Secret(username);
-
- //secret fetched on both attempts must be same
- assertEquals(attempt1.getAwsSecret(), attempt2.getAwsSecret());
-
- //access key fetched on both attempts must be same
- assertEquals(attempt1.getAwsAccessKey(), attempt2.getAwsAccessKey());
-
Review Comment:
Rather than simply removing the second `getS3Secret()`, we should confirm it
now throws expected exception:
```java
// A second getS3Secret on the same username should throw exception
LambdaTestUtils.intercept(OMException.class,
"expected error message",
() -> omClient.getS3Secret(username));
```
--
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]
