You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "david1859168 (via GitHub)" <gi...@apache.org> on 2023/12/22 05:47:01 UTC
[PR] HDDS-9503. Migrate simple misc. integration tests to JUnit5 [ozone]
david1859168 opened a new pull request, #5854:
URL: https://github.com/apache/ozone/pull/5854
## What changes were proposed in this pull request?
HDDS-9503. Migrate simple misc. integration tests to JUnit5
Please describe your PR in detail:
Migrate the following test classes from Junit4 to JUnit5 by following the parent JIRA [HDDS-6729. Migrate tests to JUnit5]
hadoop-ozone/fault-injection-test/mini-chaos-tests/src/test/java/org/apache/hadoop/ozone/TestMiniChaosOzoneCluster.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/storage/TestCommitWatcher.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokens.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokensCLI.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerOperations.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerStateMachineIdempotency.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneOMHACluster.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/debug/TestLeaseRecoverer.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/fsck/TestContainerMapper.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestContainerReportWithKeys.java
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestSecretKeysApi.java
## How was this patch tested?
Tested in Intellij.
--
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-9503. Migrate simple misc. integration tests to JUnit5 [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5854:
URL: https://github.com/apache/ozone/pull/5854#issuecomment-1867355243
Thanks @david1859168 for working on this.
There are some conflicts with current `master` branch, which need to be resolved manually. Please merge from `master`, resolve the conflicts, then push to your fork again.
Also, checkstyle reports the following problems:
```
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokens.java
53: Unused import - org.apache.ozone.test.JUnit5AwareTimeout.
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerOperations.java
32: Using the '.*' form of import should be avoided - org.junit.jupiter.api.*.
53: 'class def modifier' has incorrect indentation level 1, expected level should be 0.
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneOMHACluster.java
24: Using the '.*' form of import should be avoided - org.junit.jupiter.api.*.
27: Unused import - java.io.IOException.
48: Variable 'exception' must be private and have accessor methods.
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokensCLI.java
36: Using the '.*' form of import should be avoided - org.junit.jupiter.api.*.
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestSecretKeysApi.java
42: Unused import - org.apache.ozone.test.JUnit5AwareTimeout.
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestContainerReportWithKeys.java
40: Using the '.*' form of import should be avoided - org.junit.jupiter.api.*.
42: Unused import - org.apache.ozone.test.JUnit5AwareTimeout.
69: Variable 'exception' must be private and have accessor methods.
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerStateMachineIdempotency.java
40: Using the '.*' form of import should be avoided - org.junit.jupiter.api.*.
```
https://github.com/david1859168/ozone/actions/runs/7296598719/job/19884620249
You can run checkstyle locally by:
```
hadoop-ozone/dev-support/checks/checkstyle.sh
```
--
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-9503. Migrate simple misc. integration tests to JUnit5 [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #5854:
URL: https://github.com/apache/ozone/pull/5854
--
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-9503. Migrate simple misc. integration tests to JUnit5 [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5854:
URL: https://github.com/apache/ozone/pull/5854#issuecomment-1867754007
Thanks again @david1859168 for the patch.
--
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-9503. Migrate simple misc. integration tests to JUnit5 [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5854:
URL: https://github.com/apache/ozone/pull/5854#discussion_r1434956568
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestContainerReportWithKeys.java:
##########
@@ -66,7 +68,7 @@ public class TestContainerReportWithKeys {
private static OzoneConfiguration conf;
private static StorageContainerManager scm;
- public ExpectedException exception = ExpectedException.none();
+ private ExpectedException exception = ExpectedException.none();
Review Comment:
Sorry, I missed this one earlier. `ExpectedException` is no longer needed.
```suggestion
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestContainerReportWithKeys.java:
##########
@@ -37,12 +37,14 @@
import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
-import org.junit.jupiter.api.*;
import org.junit.rules.ExpectedException;
Review Comment:
```suggestion
```
--
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-9503. Migrate simple misc. integration tests to JUnit5 [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
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: 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