Copilot commented on code in PR #8200:
URL: https://github.com/apache/hbase/pull/8200#discussion_r3221739051
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSimpleTimeRangeTracker.java:
##########
@@ -17,29 +17,24 @@
*/
package org.apache.hadoop.hbase.regionserver;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.DataOutputStream;
import java.io.IOException;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.io.ByteArrayOutputStream;
import org.apache.hadoop.hbase.io.TimeRange;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
Review Comment:
This change reclassifies the test from `SmallTests` to `MediumTests`, which
is a behavioral change in test categorization and looks unrelated to the stated
goal of the PR (JUnit 5 migration). If this was not intentional, keep the
previous test classification/tag (i.e., `SmallTests`) while migrating to JUnit
Jupiter.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSimpleTimeRangeTracker.java:
##########
@@ -17,29 +17,24 @@
*/
package org.apache.hadoop.hbase.regionserver;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.DataOutputStream;
import java.io.IOException;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.io.ByteArrayOutputStream;
import org.apache.hadoop.hbase.io.TimeRange;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
-import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
-import org.junit.ClassRule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
-@Category({ RegionServerTests.class, SmallTests.class })
+@Tag(RegionServerTests.TAG)
+@Tag(MediumTests.TAG)
Review Comment:
This change reclassifies the test from `SmallTests` to `MediumTests`, which
is a behavioral change in test categorization and looks unrelated to the stated
goal of the PR (JUnit 5 migration). If this was not intentional, keep the
previous test classification/tag (i.e., `SmallTests`) while migrating to JUnit
Jupiter.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSettingTimeoutOnBlockingPoint.java:
##########
@@ -69,7 +68,7 @@ public static void setUpBeforeClass() throws Exception {
TEST_UTIL.startMiniCluster(2);
}
- @AfterClass
+ @AfterAll
public static void setUpAfterClass() throws Exception {
Review Comment:
This method is now annotated with `@AfterAll` but is still named
`setUpAfterClass`. Renaming it to something like `tearDownAfterClass` would
better reflect its purpose and avoid confusion during future maintenance.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileScannerWithTagCompression.java:
##########
@@ -43,25 +42,20 @@
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
-@Category({ RegionServerTests.class, SmallTests.class })
[email protected](RegionServerTests.TAG)
[email protected](SmallTests.TAG)
Review Comment:
The new `@Tag` usage is fully-qualified here, while the rest of the migrated
tests import `org.junit.jupiter.api.Tag` and use `@Tag(...)`. For consistency
and readability, add the `Tag` import and switch these annotations to
`@Tag(RegionServerTests.TAG)` / `@Tag(SmallTests.TAG)`.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSecureBulkloadListener.java:
##########
@@ -87,15 +81,14 @@ public void setUp() throws Exception {
dfs = (DistributedFileSystem) FileSystem.get(conf);
}
- @After
+ @AfterEach
public void tearDownAfterClass() throws Exception {
Review Comment:
This method is annotated `@AfterEach` but named `tearDownAfterClass`, which
is misleading (it runs after every test, not after the class). Rename it to
`tearDown`/`afterEach` (or switch to `@AfterAll` if the intent really is
once-per-class cleanup).
--
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]
