Copilot commented on code in PR #8301:
URL: https://github.com/apache/hbase/pull/8301#discussion_r3340595710
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestSyncReplicationActiveFSHLog.java:
##########
@@ -17,25 +17,22 @@
*/
package org.apache.hadoop.hbase.replication;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.testclassification.ReplicationTests;
import org.apache.hadoop.hbase.wal.WALFactory;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+@Tag(ReplicationTests.TAG)
+@Tag(LargeTests.TAG)
@Category({ ReplicationTests.class, LargeTests.class })
public class TestSyncReplicationActiveFSHLog extends
SyncReplicationActiveTestBase {
Review Comment:
The JUnit 4 `@Category` annotation and its import
(`org.junit.experimental.categories.Category`) were not removed when adding the
JUnit 5 `@Tag` annotations. The class is now double-annotated with both tagging
systems.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationChangingPeerRegionservers.java:
##########
@@ -80,12 +74,12 @@ protected boolean isSyncPeer() {
}
@Parameters(name = "{index}: serialPeer={0}, syncPeer={1}")
Review Comment:
The leftover JUnit 4 `@Parameters` annotation (and its
`org.junit.runners.Parameterized.Parameters` import on line 42) is no longer
effective now that the class uses `@HBaseParameterizedTestTemplate`. The
parameter naming pattern is already provided through
`@HBaseParameterizedTestTemplate(name = ...)`. The unused annotation/import
should be removed for clarity.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestClaimReplicationQueue.java:
##########
@@ -100,33 +97,32 @@ protected ServerManager createServerManager(MasterServices
master, RegionServerL
}
}
- @BeforeClass
+ @BeforeAll
public static void setUpBeforeClass() throws Exception {
CONF1.setClass(HConstants.MASTER_IMPL, HMasterForTest.class,
HMaster.class);
- TestReplicationBase.setUpBeforeClass();
+ configureClusters(UTIL1, UTIL2);
+ startClusters();
createTable(tableName3);
table3 = connection1.getTable(tableName3);
table4 = connection2.getTable(tableName3);
}
- @AfterClass
+ @AfterAll
public static void tearDownAfterClass() throws Exception {
Closeables.close(table3, true);
Closeables.close(table4, true);
TestReplicationBase.tearDownAfterClass();
Review Comment:
This class no longer extends `TestReplicationBase`; it extends
`TestReplicationBaseNoBeforeAll` directly. The explicit call
`TestReplicationBase.tearDownAfterClass()` still resolves via inheritance, but
it is misleading because the call appears to delegate to an unrelated class.
Consider calling `TestReplicationBaseNoBeforeAll.tearDownAfterClass()` instead,
to match the actual parent class.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSmallTests.java:
##########
@@ -390,7 +384,7 @@ public void testVerifyListReplicatedTable() throws
Exception {
// check the matching result
for (int i = 0; i < match.length; i++) {
- assertTrue("listReplicated() does not match table " + i, (match[i] ==
1));
+ assertEquals(match[i], 1, "listReplicated() does not match table " + i);
Review Comment:
The expected and actual arguments are swapped. JUnit 5's `assertEquals`
signature is `assertEquals(expected, actual, message)`, but here the literal
expected value `1` is passed as the actual argument and `match[i]` is passed as
expected. On failure the diagnostic message will report the values reversed.
The original JUnit 4 code used `assertTrue("...", match[i] == 1)`, so the
equivalent JUnit 5 form should keep `1` as the expected value.
--
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]
