Copilot commented on code in PR #8009:
URL: https://github.com/apache/hbase/pull/8009#discussion_r3014322912
##########
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 in Mockito 4.x in favor
of `openMocks`. Please switch to `MockitoAnnotations.openMocks(this)` (and
close the returned AutoCloseable in `@AfterEach`), or use
`@ExtendWith(MockitoExtension.class)` for JUnit5.
##########
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 is still largely using JUnit4 (imports,
@BeforeClass/@Before/@Test, @Category, @ClassRule), but the teardown method was
switched to JUnit5's @AfterEach. Under the JUnit4 Vintage engine, `@AfterEach`
will not run, so `region.close(true)` will be skipped and resources can leak
across tests. Please migrate the whole class to JUnit5 (use
@BeforeAll/@BeforeEach/@AfterEach + `@Tag` and remove JUnit4 rules), or keep
JUnit4 annotations consistently (revert to @After).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/http/TestRegionVisualizer.java:
##########
@@ -76,7 +70,6 @@ public void testRegionDetailsJsonSerialization() throws
Exception {
final Gson gson = RegionVisualizer.buildGson();
final JsonObject result = gson.fromJson(gson.toJson(regionDetails),
JsonObject.class);
- Assert.assertNotNull(result);
assertEquals(serverName.toShortString(),
result.get("server_name").getAsString());
assertEquals(tableName.getNameAsString(),
result.get("table_name").getAsString());
Review Comment:
`result` is dereferenced immediately; if JSON serialization unexpectedly
returns null, the test will fail with an NPE instead of a clear assertion
failure. Add an `assertNotNull(result)` before accessing fields so failures are
easier to diagnose.
##########
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 (typically in an `@AfterEach`) to avoid leaking mock resources across
tests. Consider switching to `@ExtendWith(MockitoExtension.class)` or storing
the returned AutoCloseable and closing it during teardown.
##########
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 HBaseCommonTestingUtil();
Review Comment:
`@ExtendWith(MockitoExtension.class)` already initializes the `@Mock` fields
for each test. Calling `MockitoAnnotations.openMocks(this)` in `@BeforeEach` is
redundant and can also leak resources since the returned AutoCloseable is never
closed. Remove the `openMocks` call and rely on `MockitoExtension`, or (if you
prefer manual init) drop the extension and close the AutoCloseable in
`@AfterEach`.
--
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]
