Copilot commented on code in PR #8132:
URL: https://github.com/apache/hbase/pull/8132#discussion_r3139161430
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedureFileBasedSFT.java:
##########
@@ -20,24 +20,14 @@
import static
org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
import static
org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.Trackers.FILE;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.testclassification.MediumTests;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
-import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.Tag;
-@Category({ MasterTests.class, MediumTests.class })
+@Tag(MasterTests.TAG)
+@Tag(MediumTests.TAG)
public class TestCloneSnapshotProcedureFileBasedSFT extends
TestCloneSnapshotProcedure {
-
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestCloneSnapshotProcedureFileBasedSFT.class);
-
- @BeforeClass
- public static void setupCluster() throws Exception {
+ static {
UTIL.getConfiguration().set(TRACKER_IMPL, FILE.name());
-
UTIL.getConfiguration().setInt(MasterProcedureConstants.MASTER_PROCEDURE_THREADS,
1);
- UTIL.startMiniCluster(1);
}
Review Comment:
The static initializer mutates `TestTableDDLProcedureBase.UTIL`
configuration (TRACKER_IMPL) but never restores the previous value. Since
`UTIL` is a shared static across all subclasses, this can bleed into other
tests depending on execution order. Consider saving the old value and restoring
it in an `@AfterAll`, or isolating this test by using a dedicated
`HBaseTestingUtility`/`Configuration` instance for this subclass.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestWALProcedureStoreOnHDFS.java:
##########
@@ -109,7 +105,7 @@ public void tearDown() throws Exception {
}
}
Review Comment:
`tearDown()` is neither annotated with `@AfterEach` nor invoked by the
tests, so the MiniDFS cluster/WAL store started in `setupDFS()` will not be
cleaned up. This can leak resources and cause cross-test
interference/flakiness. Consider adding `@AfterEach` (and ensuring it tolerates
partial initialization), or calling `tearDown()` in a `finally` block from each
test that calls `setupDFS()`.
--
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]
