Copilot commented on code in PR #8199:
URL: https://github.com/apache/hbase/pull/8199#discussion_r3221595077
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRecoveredEdits.java:
##########
@@ -232,8 +229,8 @@ public static int verifyAllEditsMadeItIn(final FileSystem
fs, final Configuratio
i++;
}
}
- assertEquals("Only found " + found + " cells in region, but there are " +
walCells.size()
- + " cells in recover edits", found, walCells.size());
+ assertEquals(found, walCells.size(), "Only found " + found + " cells in
region, but there are "
Review Comment:
The `assertEquals` argument order is inverted relative to the intent
described by the message. `found` is the actual count and `walCells.size()` is
the expected count; swapping them will make failure output (expected vs actual)
accurate and easier to diagnose.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithModifyTable.java:
##########
@@ -17,67 +17,64 @@
*/
package org.apache.hadoop.hbase.regionserver;
-import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import java.io.IOException;
-import java.util.Arrays;
-import java.util.List;
+import java.util.stream.Stream;
import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseParameterizedTestTemplate;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
-import org.apache.hadoop.hbase.TableNameTestRule;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.RegionSplitter;
-import org.junit.After;
-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.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-import org.junit.runners.Parameterized.Parameters;
-
-@RunWith(Parameterized.class)
-@Category({ RegionServerTests.class, MediumTests.class })
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.params.provider.Arguments;
+
+@Tag(RegionServerTests.TAG)
+@Tag(MediumTests.TAG)
+@HBaseParameterizedTestTemplate(name = "{index}: disableBeforeModifying={0}")
public class TestRegionReplicasWithModifyTable {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestRegionReplicasWithModifyTable.class);
-
private static final int NB_SERVERS = 3;
private static final HBaseTestingUtil HTU = new HBaseTestingUtil();
private static final byte[] f = HConstants.CATALOG_FAMILY;
- @Parameter
- public boolean disableBeforeModifying;
+ private final boolean disableBeforeModifying;
+
+ private TableName tableName;
- @Rule
- public TableNameTestRule name = new TableNameTestRule();
+ public static Stream<Arguments> parameters() {
+ return Stream.of(Arguments.of(true), Arguments.of(false));
+ }
- @Parameters
- public static List<Object[]> params() {
- return Arrays.asList(new Object[] { true }, new Object[] { false });
+ public TestRegionReplicasWithModifyTable(boolean disableBeforeModifying) {
+ this.disableBeforeModifying = disableBeforeModifying;
}
- @BeforeClass
+ @BeforeAll
public static void before() throws Exception {
HTU.startMiniCluster(NB_SERVERS);
}
+ @BeforeEach
+ public void setUp(TestInfo testInfo) {
+ tableName = TableName.valueOf(testInfo.getTestMethod().get().getName());
Review Comment:
This test is now a JUnit5 test template with two parameter invocations, but
`tableName` is derived only from the method name. That means both invocations
for the same `@TestTemplate` will target the same table name, which can cause
collisions/flakiness (especially if template invocations run concurrently or if
one invocation fails before cleanup). Consider incorporating the parameter
value or invocation index into the table name (e.g., include
`disableBeforeModifying` or a sanitized `testInfo.getDisplayName()`), so each
invocation gets a unique table.
--
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]
