Copilot commented on code in PR #8180:
URL: https://github.com/apache/hbase/pull/8180#discussion_r3172331500
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java:
##########
@@ -21,24 +21,26 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
import org.apache.hadoop.hbase.wal.WAL;
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;
+import org.junit.jupiter.api.TestInfo;
-@Category({ RegionServerTests.class, MediumTests.class })
+@Tag(RegionServerTests.TAG)
+@Tag(MediumTests.TAG)
public class TestWALReplay extends AbstractTestWALReplay {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestWALReplay.class);
+ @BeforeAll
+ public static void setUpBeforeClass(TestInfo testInfo) throws Exception {
+ if (testInfo.getTestClass().get() == TestWALReplay.class) {
+ setUpBeforeClass();
+ }
+ }
Review Comment:
`@BeforeAll` methods cannot be `static` with injected parameters like
`TestInfo` (JUnit Jupiter does not support parameter injection for static
lifecycle methods). This will fail test discovery/execution. Remove this
overload and just annotate the existing no-arg `setUpBeforeClass()` (or make
the `@BeforeAll` non-static with `@TestInstance(PER_CLASS)`).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncWALReplay.java:
##########
@@ -21,37 +21,39 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.io.asyncfs.monitor.StreamSlowMonitor;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
import org.apache.hadoop.hbase.util.Threads;
import org.apache.hadoop.hbase.wal.WAL;
import org.apache.hadoop.hbase.wal.WALFactory;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
-import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.TestInfo;
import
org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
import org.apache.hbase.thirdparty.io.netty.channel.Channel;
import org.apache.hbase.thirdparty.io.netty.channel.EventLoopGroup;
import org.apache.hbase.thirdparty.io.netty.channel.nio.NioEventLoopGroup;
import
org.apache.hbase.thirdparty.io.netty.channel.socket.nio.NioSocketChannel;
-@Category({ RegionServerTests.class, MediumTests.class })
+@Tag(RegionServerTests.TAG)
+@Tag(MediumTests.TAG)
public class TestAsyncWALReplay extends AbstractTestWALReplay {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestAsyncWALReplay.class);
-
private static EventLoopGroup GROUP;
private static Class<? extends Channel> CHANNEL_CLASS;
- @BeforeClass
+ @BeforeAll
+ public static void setUpBeforeClass(TestInfo testInfo) throws Exception {
+ if (testInfo.getTestClass().get() == TestAsyncWALReplay.class) {
+ setUpBeforeClass();
+ }
+ }
Review Comment:
`@BeforeAll` is static here but declares a `TestInfo` parameter. JUnit
Jupiter does not support parameter injection for static `@BeforeAll` methods,
so this will throw a ParameterResolutionException at runtime. Prefer annotating
the existing no-arg `setUpBeforeClass()` (or switch to non-static `@BeforeAll`
with `@TestInstance(PER_CLASS)`).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/AbstractTestFSWAL.java:
##########
@@ -391,20 +391,19 @@ public void testFindMemStoresEligibleForFlush() throws
Exception {
}
- @Test(expected = IOException.class)
+ @Test
public void testFailedToCreateWALIfParentRenamed()
throws IOException, CommonFSUtils.StreamLacksCapabilityException {
- AbstractFSWAL<?> wal = newWAL(FS, CommonFSUtils.getWALRootDir(CONF),
- currentTest.getMethodName(), HConstants.HREGION_OLDLOGDIR_NAME, CONF,
null, true, null, null);
+ AbstractFSWAL<?> wal = newWAL(FS, CommonFSUtils.getWALRootDir(CONF),
testName,
+ HConstants.HREGION_OLDLOGDIR_NAME, CONF, null, true, null, null);
long filenum = EnvironmentEdgeManager.currentTime();
Path path = wal.computeFilename(filenum);
wal.createWriterInstance(FS, path);
Path parent = path.getParent();
- path = wal.computeFilename(filenum + 1);
Path newPath = new Path(parent.getParent(), parent.getName() +
"-splitting");
FS.rename(parent, newPath);
- wal.createWriterInstance(FS, path);
- fail("It should fail to create the new WAL");
+ Path nextPath = wal.computeFilename(filenum + 1);
+ assertThrows(IOException.class, () -> wal.createWriterInstance(FS,
nextPath));
Review Comment:
`wal` is created but never closed in this test. With the switch to
`assertThrows`, the test now completes normally, so the WAL will remain open
and can leak threads/FS resources into subsequent tests. Wrap the WAL creation
in try-with-resources or close it in a finally block.
--
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]
