adoroszlai commented on code in PR #5854:
URL: https://github.com/apache/ozone/pull/5854#discussion_r1434876213
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerOperations.java:
##########
@@ -29,39 +29,35 @@
import org.apache.hadoop.hdds.scm.ha.SCMHAUtils;
import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
import org.apache.hadoop.hdds.scm.pipeline.PipelineNotFoundException;
-import org.junit.AfterClass;
-import org.junit.Assert;
-import org.junit.BeforeClass;
-import org.junit.Test;
-import org.junit.Rule;
-import org.junit.rules.TestRule;
-import org.junit.rules.Timeout;
-import org.apache.ozone.test.JUnit5AwareTimeout;
+import org.junit.jupiter.api.*;
import java.io.IOException;
import java.util.List;
+import java.util.concurrent.TimeUnit;
import static
org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.REPLICATION;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* This class tests container operations (TODO currently only supports create)
* from cblock clients.
*/
-public class TestContainerOperations {
- /**
- * Set a timeout for each test.
- */
- @Rule
- public TestRule timeout = new JUnit5AwareTimeout(Timeout.seconds(300));
+/**
+ * Set a timeout for each test.
+ */
+ /**
+ * Set a timeout for each test.
+ */
+ @Timeout(value = 300, unit = TimeUnit.SECONDS)
Review Comment:
`@Timeout` has an accidental indentation.
```suggestion
@Timeout(value = 300, unit = TimeUnit.SECONDS)
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokens.java:
##########
@@ -206,7 +204,7 @@ public void blockTokenFailsOnExpiredSecretKey() throws
Exception {
StorageContainerException ex =
assertThrows(StorageContainerException.class,
() -> readDataWithoutRetry(keyInfo));
assertEquals(BLOCK_TOKEN_VERIFICATION_FAILED, ex.getResult());
- assertThat(ex).hasMessageContaining("Token can't be verified due to
expired secret key");
+ assertTrue(ex.getMessage().contains("Token can't be verified due to
expired secret key"));
Review Comment:
Please don't revert to the code prior to 0008d9ab086.
```suggestion
assertThat(ex).hasMessageContaining("Token can't be verified due to
expired secret key");
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokensCLI.java:
##########
@@ -33,14 +33,7 @@
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.ozone.test.GenericTestUtils;
import org.apache.ratis.util.ExitUtils;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.jupiter.api.Assertions;
-import org.junit.rules.TestRule;
-import org.junit.rules.Timeout;
-import org.apache.ozone.test.JUnit5AwareTimeout;
+import org.junit.jupiter.api.*;
Review Comment:
Please avoid wildcard import, import single classes.
If you use IDEA, it should be configured to not replace single imports
automatically:
https://www.jetbrains.com/help/idea/2023.3/creating-and-optimizing-imports.html#disable-wildcard-imports
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokens.java:
##########
@@ -252,7 +250,7 @@ public void blockTokenFailsOnWrongSecretKeyId() throws
Exception {
assertThrows(StorageContainerException.class,
() -> readDataWithoutRetry(keyInfo));
assertEquals(BLOCK_TOKEN_VERIFICATION_FAILED, ex.getResult());
- assertThat(ex).hasMessageContaining("Can't find the signing secret key");
+ assertTrue(ex.getMessage().contains("Can't find the signing secret key"));
Review Comment:
```suggestion
assertThat(ex).hasMessageContaining("Can't find the signing secret key");
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java:
##########
@@ -26,37 +26,36 @@
import org.apache.ozone.test.UnhealthyTest;
import org.apache.ozone.test.tag.Unhealthy;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
import org.junit.experimental.categories.Category;
-import org.junit.rules.TestRule;
-import org.junit.rules.Timeout;
-import org.apache.ozone.test.JUnit5AwareTimeout;
+import org.junit.jupiter.api.Timeout;
import java.util.Optional;
+import java.util.concurrent.TimeUnit;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* This class tests container balancer operations
* from cblock clients.
*/
+
+/**
+ * Set a timeout for each test.
+ */
+@Timeout(value = 300, unit = TimeUnit.MILLISECONDS)
+
Review Comment:
The javadoc comment, which was previously on the `TestRule timeout` instance
variable, is no longer applicable.
```suggestion
@Timeout(value = 300, unit = TimeUnit.MILLISECONDS)
```
(The same applies to other classes, too.)
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokens.java:
##########
@@ -47,13 +47,11 @@
import org.apache.hadoop.security.token.Token;
import org.apache.ozone.test.GenericTestUtils;
import org.apache.ratis.util.ExitUtils;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TestRule;
-import org.junit.rules.Timeout;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
import org.apache.ozone.test.JUnit5AwareTimeout;
Review Comment:
`JUnit5AwareTimeout` is no longer used in the class, please remove the
import. (Also in other classes.)
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokens.java:
##########
@@ -275,7 +273,7 @@ public void blockTokenFailsOnWrongPassword() throws
Exception {
assertThrows(StorageContainerException.class,
() -> readDataWithoutRetry(keyInfo));
assertEquals(BLOCK_TOKEN_VERIFICATION_FAILED, ex.getResult());
- assertThat(ex).hasMessageContaining("Invalid token for user");
+ assertTrue(ex.getMessage().contains("Invalid token for user"));
Review Comment:
```suggestion
assertThat(ex).hasMessageContaining("Invalid token for user"));
```
--
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]
