Copilot commented on code in PR #8151:
URL: https://github.com/apache/hbase/pull/8151#discussion_r3147731972
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALCellCodecWithCompression.java:
##########
@@ -17,78 +17,71 @@
*/
package org.apache.hadoop.hbase.regionserver.wal;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;
+import java.util.stream.Stream;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.ArrayBackedTag;
import org.apache.hadoop.hbase.ByteBufferKeyValue;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseCommonTestingUtil;
+import org.apache.hadoop.hbase.HBaseParameterizedTestTemplate;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.KeyValue;
import org.apache.hadoop.hbase.PrivateCellUtil;
-import org.apache.hadoop.hbase.Tag;
import org.apache.hadoop.hbase.codec.Codec.Decoder;
import org.apache.hadoop.hbase.codec.Codec.Encoder;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.io.util.LRUDictionary;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.util.Bytes;
-import org.junit.ClassRule;
-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.Parameters;
-
-@Category({ RegionServerTests.class, SmallTests.class })
-@RunWith(Parameterized.class)
-public class TestWALCellCodecWithCompression {
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.params.provider.Arguments;
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestWALCellCodecWithCompression.class);
+@Tag(RegionServerTests.TAG)
+@Tag(SmallTests.TAG)
+@HBaseParameterizedTestTemplate
+public class TestWALCellCodecWithCompression {
- private Compression.Algorithm compression;
+ private final Compression.Algorithm compression;
public TestWALCellCodecWithCompression(Compression.Algorithm algo) {
this.compression = algo;
}
- @Parameters
- public static List<Object[]> params() {
- return HBaseCommonTestingUtil.COMPRESSION_ALGORITHMS_PARAMETERIZED;
+ public static Stream<Arguments> parameters() {
+ return
HBaseCommonTestingUtil.COMPRESSION_ALGORITHMS_PARAMETERIZED.stream().map(Arguments::of);
}
- @Test
+ @TestTemplate
public void testEncodeDecodeKVsWithTags() throws Exception {
doTest(false, false);
}
- @Test
+ @TestTemplate
public void testEncodeDecodeKVsWithTagsWithTagsCompression() throws
Exception {
doTest(true, false);
}
- @Test
+ @TestTemplate
public void testEncodeDecodeOffKVsWithTagsWithTagsCompression() throws
Exception {
doTest(true, false);
}
Review Comment:
`testEncodeDecodeOffKVsWithTagsWithTagsCompression` currently calls
`doTest(true, false)`, which exercises the on-heap KV path. Given the test name
and `doTest(..., offheapKV)` signature, this should likely pass `true` for the
`offheapKV` argument so the off-heap encoding/decoding path is actually covered.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALConfiguration.java:
##########
@@ -17,58 +17,55 @@
*/
package org.apache.hadoop.hbase.regionserver.wal;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
import java.io.IOException;
-import java.util.Arrays;
+import java.util.stream.Stream;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseParameterizedTestTemplate;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.wal.WAL;
import org.apache.hadoop.hbase.wal.WALFactory;
import org.apache.hadoop.hbase.wal.WALProvider;
-import org.junit.Before;
-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.runner.RunWith;
-import org.junit.runners.Parameterized;
+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;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* Ensure configuration changes are having an effect on WAL. There is a lot of
reflection around WAL
* setup; could be skipping Configuration changes.
*/
-@RunWith(Parameterized.class)
-@Category({ RegionServerTests.class, SmallTests.class })
+@Tag(RegionServerTests.TAG)
+@Tag(SmallTests.TAG)
+@HBaseParameterizedTestTemplate(name = "{index}: provider={0}")
public class TestWALConfiguration {
private static final Logger LOG =
LoggerFactory.getLogger(TestWALConfiguration.class);
static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestWALConfiguration.class);
- @Rule
- public TestName name = new TestName();
+ private String name;
- @Parameterized.Parameter
- public String walProvider;
+ private final String walProvider;
Review Comment:
`name` is a private field but it is never used in this class (only
assigned). Please remove it to avoid dead code in tests.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALConfiguration.java:
##########
@@ -17,58 +17,55 @@
*/
package org.apache.hadoop.hbase.regionserver.wal;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
import java.io.IOException;
-import java.util.Arrays;
+import java.util.stream.Stream;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseParameterizedTestTemplate;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.wal.WAL;
import org.apache.hadoop.hbase.wal.WALFactory;
import org.apache.hadoop.hbase.wal.WALProvider;
-import org.junit.Before;
-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.runner.RunWith;
-import org.junit.runners.Parameterized;
+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;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* Ensure configuration changes are having an effect on WAL. There is a lot of
reflection around WAL
* setup; could be skipping Configuration changes.
*/
-@RunWith(Parameterized.class)
-@Category({ RegionServerTests.class, SmallTests.class })
+@Tag(RegionServerTests.TAG)
+@Tag(SmallTests.TAG)
+@HBaseParameterizedTestTemplate(name = "{index}: provider={0}")
public class TestWALConfiguration {
private static final Logger LOG =
LoggerFactory.getLogger(TestWALConfiguration.class);
static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestWALConfiguration.class);
- @Rule
- public TestName name = new TestName();
+ private String name;
- @Parameterized.Parameter
- public String walProvider;
+ private final String walProvider;
- @Parameterized.Parameters(name = "{index}: provider={0}")
- public static Iterable<Object[]> data() {
- return Arrays.asList(new Object[] { "filesystem" }, new Object[] {
"asyncfs" });
+ public static Stream<Arguments> parameters() {
+ return Stream.of(Arguments.of("filesystem"), Arguments.of("asyncfs"));
}
- @Before
- public void before() {
+ public TestWALConfiguration(String walProvider) {
+ this.walProvider = walProvider;
+ }
+
+ @BeforeEach
+ public void before(TestInfo testInfo) {
+ name = testInfo.getTestMethod().get().getName();
TEST_UTIL.getConfiguration().set(WALFactory.WAL_PROVIDER, walProvider);
Review Comment:
This assignment sets `name`, but the value is not used anywhere afterwards.
After removing the unused field, this line should also be removed.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestSyncFuture.java:
##########
@@ -17,31 +17,29 @@
*/
package org.apache.hadoop.hbase.regionserver.wal;
-import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
-import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.exceptions.TimeoutIOException;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
-import org.junit.ClassRule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
-@Category({ RegionServerTests.class, SmallTests.class })
+@Tag(RegionServerTests.TAG)
+@Tag(SmallTests.TAG)
public class TestSyncFuture {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestSyncFuture.class);
-
- @Test(expected = TimeoutIOException.class)
+ @Test
public void testGet() throws Exception {
- long timeout = 5000;
- long txid = 100000;
- SyncFuture syncFulture = new SyncFuture().reset(txid, false);
- syncFulture.done(txid, null);
- assertEquals(txid, syncFulture.get(timeout));
+ assertThrows(TimeoutIOException.class, () -> {
+ long timeout = 5000;
+ long txid = 100000;
+ SyncFuture syncFulture = new SyncFuture().reset(txid, false);
+ syncFulture.done(txid, null);
+ assertEquals(txid, syncFulture.get(timeout));
Review Comment:
Typo in local variable name: `syncFulture` should be `syncFuture` for
readability and to avoid propagating misspellings.
--
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]
