Copilot commented on code in PR #7960:
URL: https://github.com/apache/hbase/pull/7960#discussion_r2964779866
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/RSGroupableBalancerTestBase.java:
##########
@@ -17,9 +17,10 @@
*/
package org.apache.hadoop.hbase.master.balancer;
-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 static org.mockito.Mockito.any;
Review Comment:
`import static org.mockito.Mockito.any;` is not the correct matcher import
(matchers live under `org.mockito.ArgumentMatchers`). This will not compile and
also makes the intent less clear. Switch the static import to
`org.mockito.ArgumentMatchers.any` (or use `anyString`/`any(Class)` as
appropriate).
```suggestion
import static org.mockito.ArgumentMatchers.any;
```
##########
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 class now uses JUnit Jupiter's `@AfterEach` but still appears to rely
on JUnit 4 annotations/imports elsewhere (e.g., `org.junit.Before`,
`@Category`, etc.). Mixing JUnit 4 and JUnit 5 lifecycle annotations can result
in setup/teardown methods not running (depending on which engine executes the
test). Please migrate the remaining JUnit 4 annotations to JUnit Jupiter
equivalents (e.g., `@BeforeEach`, `@AfterAll`, `@Tag`) or keep the class purely
JUnit 4 (use `@After`).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/http/TestHbckMetricsResource.java:
##########
@@ -395,11 +397,11 @@ public void testGetUnkownServers() {
containsString(regionId), containsString(tableName),
containsString(serverName),
containsString(serverName), containsString(port),
containsString(startCode),
containsString(metaRegionID), containsString(metaTableName),
containsString(localhost1),
- containsString(localhost2), containsString(port),
containsString(startCode)));
+ containsString(localhost2), containsString(port),
containsString(hostStartCode)));
}
Review Comment:
The test method for the JSON response is still named `testGetUnkownServers`
(typo: missing 'n' in 'Unknown'). Consider renaming it to
`testGetUnknownServers` for consistency with the HTML variant and to improve
readability/searchability.
##########
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:
This test uses `@ExtendWith(MockitoExtension.class)` but also calls
`MockitoAnnotations.openMocks(this)` in `@BeforeEach`. That creates a second
Mockito session and reinitializes the `@Mock` fields, which can lead to
confusing behavior and unnecessary resources. Remove the manual `openMocks`
call and let `MockitoExtension` manage mock initialization (or drop the
extension and keep manual initialization, but not both).
--
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]
