Copilot commented on code in PR #8064:
URL: https://github.com/apache/hbase/pull/8064#discussion_r3083725558
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerManagerConfigurationObserver.java:
##########
@@ -67,7 +62,7 @@ public class TestRegionNormalizerManagerConfigurationObserver
{
private RegionNormalizerWorker worker;
private ConfigurationManager configurationManager;
- @Before
+ @BeforeEach
public void before() {
MockitoAnnotations.initMocks(this);
conf = testUtil.getConfiguration();
Review Comment:
MockitoAnnotations.initMocks(this) is deprecated and does not provide a way
to release resources. Switch to MockitoAnnotations.openMocks(this) and close it
in an @AfterEach, or use @ExtendWith(MockitoExtension.class) for JUnit 5 tests.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java:
##########
@@ -38,52 +37,44 @@
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.CommonFSUtils;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-import org.junit.rules.TestName;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
/**
* Test that the snapshot hfile cleaner finds hfiles referenced in a snapshot
*/
-@Category({ MasterTests.class, SmallTests.class })
+@Tag(MasterTests.TAG)
+@Tag(SmallTests.TAG)
public class TestSnapshotHFileCleaner {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestSnapshotHFileCleaner.class);
-
private final static HBaseTestingUtility TEST_UTIL = new
HBaseTestingUtility();
private static final String TABLE_NAME_STR = "testSnapshotManifest";
private static final String SNAPSHOT_NAME_STR =
"testSnapshotManifest-snapshot";
private static Path rootDir;
private static FileSystem fs;
private static Configuration conf;
- @Rule
- public TestName name = new TestName();
-
/**
* Setup the test environment
*/
- @BeforeClass
+ @BeforeAll
public static void setup() throws Exception {
conf = TEST_UTIL.getConfiguration();
rootDir = CommonFSUtils.getRootDir(conf);
fs = FileSystem.get(conf);
}
- @AfterClass
+ @AfterAll
public static void cleanup() throws IOException {
// cleanup
fs.delete(rootDir, true);
}
@Test
- public void testFindsSnapshotFilesWhenCleaning() throws IOException {
+ public void testFindsSnapshotFilesWhenCleaning(TestInfo testInfo) throws
IOException {
CommonFSUtils.setRootDir(conf, TEST_UTIL.getDataTestDir());
Path rootDir = CommonFSUtils.getRootDir(conf);
Path archivedHfileDir =
Review Comment:
The test resets the configured root dir to a new temp dir, but stores it in
a local variable that shadows the static field. The `@AfterAll` cleanup deletes
the original static rootDir, so the per-test root directory created here may be
left behind. Consider updating the static rootDir/fs fields after setRootDir,
or delete using CommonFSUtils.getRootDir(conf) in cleanup.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestCacheAwareLoadBalancer.java:
##########
@@ -611,7 +607,8 @@ public void
testBalancerNotThrowNPEWhenBalancerPlansIsNull() throws Exception {
* ongoing balance run (the configuration used during balancing remains the
old one) 3. Is applied
* correctly after the balance run completes
*/
- @Test(timeout = 60000)
+ @Test
+ @Timeout(600000)
public void testConfigUpdateDuringBalance() throws Exception {
Review Comment:
`@Timeout` in JUnit Jupiter is in seconds by default. @Timeout(600000) will
allow ~7 days, not 60s (JUnit4's timeout=60000ms). Use a seconds value (e.g.,
60) or specify the unit explicitly to preserve the intended timeout.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerWorker.java:
##########
@@ -101,33 +87,36 @@ public class TestRegionNormalizerWorker {
private final AtomicReference<Throwable> workerThreadThrowable = new
AtomicReference<>();
- @Before
- public void before() throws Exception {
- MockitoAnnotations.initMocks(this);
+ private TableName tableName;
+
+ @BeforeEach
+ public void before(TestInfo testInfo) throws Exception {
+ MockitoAnnotations.openMocks(this);
when(masterServices.skipRegionManagementAction(any())).thenReturn(false);
testingUtility = new HBaseCommonTestingUtility();
Review Comment:
This test uses @ExtendWith(MockitoExtension.class) but also calls
MockitoAnnotations.openMocks(this) and does not close the returned
AutoCloseable. This is redundant and can leak mock resources across tests.
Prefer relying on MockitoExtension alone, or store/close the AutoCloseable in
@AfterEach.
##########
hbase-server/pom.xml:
##########
@@ -321,7 +321,7 @@
</dependency>
<dependency>
<groupId>org.mockito</groupId>
- <artifactId>mockito-core</artifactId>
+ <artifactId>mockito-junit-jupiter</artifactId>
<scope>test</scope>
</dependency>
Review Comment:
hbase-server/pom.xml now declares mockito-junit-jupiter twice (once here and
again later under the TODO comment). This duplication can cause Maven warnings
and makes dependency management confusing. Keep a single declaration (and if
you still need mockito-core explicitly, declare that instead of duplicating
mockito-junit-jupiter).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/http/TestMetaBrowserNoCluster.java:
##########
@@ -20,40 +20,32 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
import javax.servlet.http.HttpServletRequest;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.AsyncConnection;
import org.apache.hadoop.hbase.master.RegionState;
import org.apache.hadoop.hbase.master.http.TestMetaBrowser.MockRequestBuilder;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
-import org.junit.Before;
-import org.junit.ClassRule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
-/**
- * Cluster-backed correctness tests for the functionality provided by {@link
MetaBrowser}.
- */
-@Category({ MasterTests.class, SmallTests.class })
+@Tag(MasterTests.TAG)
+@Tag(SmallTests.TAG)
public class TestMetaBrowserNoCluster {
- @ClassRule
- public static final HBaseClassTestRule testRule =
- HBaseClassTestRule.forClass(TestMetaBrowserNoCluster.class);
-
@Mock
private AsyncConnection connection;
- @Before
+ @BeforeEach
public void before() {
- MockitoAnnotations.initMocks(this);
+ MockitoAnnotations.openMocks(this);
}
Review Comment:
MockitoAnnotations.openMocks(this) returns an AutoCloseable that should be
closed to avoid leaking resources between tests. Consider using
@ExtendWith(MockitoExtension.class) or storing the AutoCloseable and closing it
in an @AfterEach.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/region/TestMasterRegionOnTwoFileSystems.java:
##########
@@ -144,7 +144,7 @@ public void setUpBeforeTest() throws IOException {
ServerName.valueOf("localhost", 12345,
EnvironmentEdgeManager.currentTime()));
}
- @After
+ @AfterEach
public void tearDownAfterTest() {
region.close(true);
Review Comment:
This test mixes JUnit4 and JUnit5 annotations/imports: it still uses JUnit4
@Test/@Before/@BeforeClass and @ClassRule, but the teardown was converted to
JUnit5 @AfterEach. When run under the Vintage engine, `@AfterEach` will be
ignored and the region will not be closed; when run under Jupiter, the JUnit4
`@Test` methods may not execute. Convert the whole class to Jupiter (Assertions
+ @BeforeEach/@AfterEach + @Tag) or keep JUnit4 annotations consistently (use
@After).
--
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]
