Copilot commented on code in PR #8065:
URL: https://github.com/apache/hbase/pull/8065#discussion_r3078688182
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java:
##########
@@ -58,54 +57,47 @@
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.util.StringUtils;
-import org.junit.AfterClass;
-import org.junit.Before;
-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.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* End-to-end test class for filesystem space quotas.
*/
-@Category(LargeTests.class)
+@Tag(LargeTests.TAG)
public class TestSpaceQuotas {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestSpaceQuotas.class);
-
private static final Logger LOG =
LoggerFactory.getLogger(TestSpaceQuotas.class);
private static final HBaseTestingUtility TEST_UTIL = new
HBaseTestingUtility();
// Global for all tests in the class
private static final AtomicLong COUNTER = new AtomicLong(0);
private static final int NUM_RETRIES = 10;
- @Rule
- public TestName testName = new TestName();
private SpaceQuotaHelperForTests helper;
- @BeforeClass
+ @BeforeAll
public static void setUp() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
SpaceQuotaHelperForTests.updateConfigForQuotas(conf);
TEST_UTIL.startMiniCluster(1);
}
- @AfterClass
+ @AfterAll
public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}
- @Before
- public void removeAllQuotas() throws Exception {
+ @BeforeEach
+ public void removeAllQuotas(TestInfo testInto) throws Exception {
final Connection conn = TEST_UTIL.getConnection();
if (helper == null) {
- helper = new SpaceQuotaHelperForTests(TEST_UTIL, testName, COUNTER);
+ helper =
+ new SpaceQuotaHelperForTests(TEST_UTIL,
testInto.getTestMethod().get().getName(), COUNTER);
}
Review Comment:
`SpaceQuotaHelperForTests` now takes a fixed `String testName`, but this
test only initializes `helper` when it is null. After the first test, `helper`
will retain the first test's name and subsequent tests can generate/read
bulkload file paths and table names using the wrong test name (e.g.,
`generateFileToLoad` uses `testName + "_files"`). Recreate `helper` on every
`@BeforeEach` with the current `TestInfo` name (or refactor helper to obtain
the current test name dynamically) instead of caching it across tests.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreWithMiniCluster.java:
##########
@@ -92,16 +82,17 @@ public static void setUp() throws Exception {
TEST_UTIL.startMiniCluster(1);
}
- @AfterClass
+ @AfterAll
public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}
- @Before
- public void removeAllQuotas() throws Exception {
+ @BeforeEach
+ public void removeAllQuotas(TestInfo testInfo) throws Exception {
final Connection conn = TEST_UTIL.getConnection();
if (helper == null) {
- helper = new SpaceQuotaHelperForTests(TEST_UTIL, testName, COUNTER);
+ helper =
+ new SpaceQuotaHelperForTests(TEST_UTIL,
testInfo.getTestMethod().get().getName(), COUNTER);
}
Review Comment:
`SpaceQuotaHelperForTests` now stores the test name as an immutable
`String`, but this class only initializes `helper` once (when null). Subsequent
tests will keep using the first test's name when helper creates tables/paths,
which can lead to resource collisions and incorrect expectations. Create a new
`SpaceQuotaHelperForTests` in every `@BeforeEach` with the current `TestInfo`
name (or refactor helper to resolve the current test name dynamically).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSuperUserQuotaPermissions.java:
##########
@@ -96,16 +87,17 @@ public static void setupMiniCluster() throws Exception {
TEST_UTIL.startMiniCluster(1);
}
- @AfterClass
+ @AfterAll
public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}
- @Before
- public void removeAllQuotas() throws Exception {
+ @BeforeEach
+ public void removeAllQuotas(TestInfo testInfo) throws Exception {
final Connection conn = TEST_UTIL.getConnection();
if (helper == null) {
- helper = new SpaceQuotaHelperForTests(TEST_UTIL, testName, COUNTER);
+ helper =
+ new SpaceQuotaHelperForTests(TEST_UTIL,
testInfo.getTestMethod().get().getName(), COUNTER);
}
Review Comment:
`SpaceQuotaHelperForTests` now accepts a fixed `String testName`, but
`helper` is only constructed when null. This means all tests after the first
will keep using the first test's name for helper-generated resources (table
names, bulkload directories, etc.), which can cause collisions or lookups in
the wrong location. Instantiate `helper` in every `@BeforeEach` using the
current `TestInfo` (or change the helper to derive the name per-test) rather
than caching it.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterQuotasObserver.java:
##########
@@ -36,47 +35,39 @@
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
import org.apache.hadoop.hbase.testclassification.MediumTests;
-import org.junit.AfterClass;
-import org.junit.Before;
-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.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
/**
* Test class for {@link MasterQuotasObserver}.
*/
-@Category(MediumTests.class)
+@Tag(MediumTests.TAG)
public class TestMasterQuotasObserver {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestMasterQuotasObserver.class);
-
private static final HBaseTestingUtility TEST_UTIL = new
HBaseTestingUtility();
private static SpaceQuotaHelperForTests helper;
- @Rule
- public TestName testName = new TestName();
-
- @BeforeClass
+ @BeforeAll
public static void setUp() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
TEST_UTIL.startMiniCluster(1);
}
- @AfterClass
+ @AfterAll
public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}
- @Before
- public void removeAllQuotas() throws Exception {
+ @BeforeEach
+ public void removeAllQuotas(TestInfo testInfo) throws Exception {
if (helper == null) {
- helper = new SpaceQuotaHelperForTests(TEST_UTIL, testName, new
AtomicLong());
+ helper = new SpaceQuotaHelperForTests(TEST_UTIL,
testInfo.getTestMethod().get().getName(),
+ new AtomicLong());
}
Review Comment:
`SpaceQuotaHelperForTests` is now constructed with a fixed `String
testName`, but the static `helper` is only initialized once (when null). That
freezes the helper's test name to the first executed test method, which can
cause later tests to reuse the wrong name for helper-generated table/path
identifiers. Recreate `helper` per test in `@BeforeEach` (using the current
`TestInfo`), or adjust `SpaceQuotaHelperForTests` so the name is not fixed at
construction.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java:
##########
@@ -58,54 +57,47 @@
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.util.StringUtils;
-import org.junit.AfterClass;
-import org.junit.Before;
-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.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* End-to-end test class for filesystem space quotas.
*/
-@Category(LargeTests.class)
+@Tag(LargeTests.TAG)
public class TestSpaceQuotas {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestSpaceQuotas.class);
-
private static final Logger LOG =
LoggerFactory.getLogger(TestSpaceQuotas.class);
private static final HBaseTestingUtility TEST_UTIL = new
HBaseTestingUtility();
// Global for all tests in the class
private static final AtomicLong COUNTER = new AtomicLong(0);
private static final int NUM_RETRIES = 10;
- @Rule
- public TestName testName = new TestName();
private SpaceQuotaHelperForTests helper;
- @BeforeClass
+ @BeforeAll
public static void setUp() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
SpaceQuotaHelperForTests.updateConfigForQuotas(conf);
TEST_UTIL.startMiniCluster(1);
}
- @AfterClass
+ @AfterAll
public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}
- @Before
- public void removeAllQuotas() throws Exception {
+ @BeforeEach
+ public void removeAllQuotas(TestInfo testInto) throws Exception {
final Connection conn = TEST_UTIL.getConnection();
Review Comment:
Minor typo in the `@BeforeEach` parameter name: `testInto` should be
`testInfo` for readability/consistency with the rest of the JUnit5 migrations
in this module.
--
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]
