liuxiaocs7 commented on code in PR #8219: URL: https://github.com/apache/hbase/pull/8219#discussion_r3254377921
########## hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/TestPKCS12FileLoader.java: ########## @@ -17,101 +17,117 @@ */ package org.apache.hadoop.hbase.io.crypto.tls; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + import java.io.IOException; import java.security.KeyStore; -import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseParameterizedTestTemplate; import org.apache.hadoop.hbase.testclassification.SecurityTests; import org.apache.hadoop.hbase.testclassification.SmallTests; -import org.junit.Assert; -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.jupiter.api.Tag; +import org.junit.jupiter.api.TestTemplate; /** * This file has been copied from the Apache ZooKeeper project. * @see <a href= * "https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/test/java/org/apache/zookeeper/common/PKCS12FileLoaderTest.java";>Base * revision</a> */ -@RunWith(Parameterized.class) -@Category({ SecurityTests.class, SmallTests.class }) +@Tag(SecurityTests.TAG) +@Tag(SmallTests.TAG) +@HBaseParameterizedTestTemplate(name = "{index}: caKeyType={0}, certKeyType={1}, keyPassword={2}") public class TestPKCS12FileLoader extends AbstractTestX509Parameterized { - @ClassRule - public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestPKCS12FileLoader.class); + public TestPKCS12FileLoader(X509KeyType caKeyType, X509KeyType certKeyType, char[] keyPassword) { + super(caKeyType, certKeyType, keyPassword); + } - @Test + @TestTemplate public void testLoadKeyStore() throws Exception { String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.PKCS12).getAbsolutePath(); KeyStore ks = new PKCS12FileLoader.Builder().setKeyStorePath(path) .setKeyStorePassword(x509TestContext.getKeyStorePassword()).build().loadKeyStore(); - Assert.assertEquals(1, ks.size()); + assertEquals(1, ks.size()); } - @Test(expected = Exception.class) - public void testLoadKeyStoreWithWrongPassword() throws Exception { - String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.PKCS12).getAbsolutePath(); - new PKCS12FileLoader.Builder().setKeyStorePath(path) - .setKeyStorePassword("wrong password".toCharArray()).build().loadKeyStore(); + @TestTemplate + public void testLoadKeyStoreWithWrongPassword() { + assertThrows(Exception.class, () -> { + String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.PKCS12).getAbsolutePath(); + new PKCS12FileLoader.Builder().setKeyStorePath(path) + .setKeyStorePassword("wrong password".toCharArray()).build().loadKeyStore(); + }); } - @Test(expected = IOException.class) - public void testLoadKeyStoreWithWrongFilePath() throws Exception { - String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.PKCS12).getAbsolutePath(); - new PKCS12FileLoader.Builder().setKeyStorePath(path + ".does_not_exist") - .setKeyStorePassword(x509TestContext.getKeyStorePassword()).build().loadKeyStore(); + @TestTemplate + public void testLoadKeyStoreWithWrongFilePath() { + assertThrows(IOException.class, () -> { + String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.PKCS12).getAbsolutePath(); Review Comment: ditto, and the remaining tests in this class ########## hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestCommonFSUtils.java: ########## @@ -113,13 +109,15 @@ public void testGetWALRootDirUsingUri() throws IOException { assertEquals(walRoot, CommonFSUtils.getWALRootDir(conf)); } - @Test(expected = IllegalStateException.class) + @Test public void testGetWALRootDirIllegalWALDir() throws IOException { - Path root = new Path("file:///hbase/root"); - Path invalidWALDir = new Path("file:///hbase/root/logroot"); - CommonFSUtils.setRootDir(conf, root); - CommonFSUtils.setWALRootDir(conf, invalidWALDir); - CommonFSUtils.getWALRootDir(conf); + assertThrows(IllegalStateException.class, () -> { Review Comment: ditto ########## hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/TestX509Util.java: ########## @@ -119,33 +116,35 @@ public void testCreateSSLContextWithCustomProtocol() throws Exception { Arrays.asList(sslContext.newEngine(byteBufAllocatorMock).getEnabledProtocols())); } - @Test(expected = SSLContextException.class) - public void testCreateSSLContextWithoutKeyStoreLocationServer() throws Exception { - conf.unset(X509Util.TLS_CONFIG_KEYSTORE_LOCATION); - X509Util.createSslContextForServer(conf); + @TestTemplate + public void testCreateSSLContextWithoutKeyStoreLocationServer() { + assertThrows(SSLContextException.class, () -> { + conf.unset(X509Util.TLS_CONFIG_KEYSTORE_LOCATION); Review Comment: ditto ########## hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestTerminatedWrapper.java: ########## @@ -50,22 +46,25 @@ public class TestTerminatedWrapper { static final byte[][] TERMINATORS = new byte[][] { new byte[] { -2 }, Bytes.toBytes("foo") }; - @Test(expected = IllegalArgumentException.class) + @Test public void testEmptyDelimiter() { - new TerminatedWrapper<>(new RawBytes(Order.ASCENDING), ""); + assertThrows(IllegalArgumentException.class, + () -> new TerminatedWrapper<>(new RawBytes(Order.ASCENDING), "")); } - @Test(expected = IllegalArgumentException.class) + @Test public void testNullDelimiter() { - new RawBytesTerminated((byte[]) null); + assertThrows(IllegalArgumentException.class, () -> new RawBytesTerminated((byte[]) null)); // new TerminatedWrapper<byte[]>(new RawBytes(), (byte[]) null); } - @Test(expected = IllegalArgumentException.class) + @Test public void testEncodedValueContainsTerm() { - final DataType<byte[]> type = new TerminatedWrapper<>(new RawBytes(Order.ASCENDING), "foo"); - final PositionedByteRange buff = new SimplePositionedMutableByteRange(16); - type.encode(buff, Bytes.toBytes("hello foobar!")); + assertThrows(IllegalArgumentException.class, () -> { + final DataType<byte[]> type = new TerminatedWrapper<>(new RawBytes(Order.ASCENDING), "foo"); Review Comment: ditto ########## hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/TestPKCS12FileLoader.java: ########## @@ -17,101 +17,117 @@ */ package org.apache.hadoop.hbase.io.crypto.tls; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + import java.io.IOException; import java.security.KeyStore; -import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseParameterizedTestTemplate; import org.apache.hadoop.hbase.testclassification.SecurityTests; import org.apache.hadoop.hbase.testclassification.SmallTests; -import org.junit.Assert; -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.jupiter.api.Tag; +import org.junit.jupiter.api.TestTemplate; /** * This file has been copied from the Apache ZooKeeper project. * @see <a href= * "https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/test/java/org/apache/zookeeper/common/PKCS12FileLoaderTest.java";>Base * revision</a> */ -@RunWith(Parameterized.class) -@Category({ SecurityTests.class, SmallTests.class }) +@Tag(SecurityTests.TAG) +@Tag(SmallTests.TAG) +@HBaseParameterizedTestTemplate(name = "{index}: caKeyType={0}, certKeyType={1}, keyPassword={2}") public class TestPKCS12FileLoader extends AbstractTestX509Parameterized { - @ClassRule - public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestPKCS12FileLoader.class); + public TestPKCS12FileLoader(X509KeyType caKeyType, X509KeyType certKeyType, char[] keyPassword) { + super(caKeyType, certKeyType, keyPassword); + } - @Test + @TestTemplate public void testLoadKeyStore() throws Exception { String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.PKCS12).getAbsolutePath(); KeyStore ks = new PKCS12FileLoader.Builder().setKeyStorePath(path) .setKeyStorePassword(x509TestContext.getKeyStorePassword()).build().loadKeyStore(); - Assert.assertEquals(1, ks.size()); + assertEquals(1, ks.size()); } - @Test(expected = Exception.class) - public void testLoadKeyStoreWithWrongPassword() throws Exception { - String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.PKCS12).getAbsolutePath(); - new PKCS12FileLoader.Builder().setKeyStorePath(path) - .setKeyStorePassword("wrong password".toCharArray()).build().loadKeyStore(); + @TestTemplate + public void testLoadKeyStoreWithWrongPassword() { + assertThrows(Exception.class, () -> { + String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.PKCS12).getAbsolutePath(); + new PKCS12FileLoader.Builder().setKeyStorePath(path) + .setKeyStorePassword("wrong password".toCharArray()).build().loadKeyStore(); Review Comment: Can the scope included by assertThrows be reduced here? ########## hbase-common/src/test/java/org/apache/hadoop/hbase/io/compress/TestCodecPool.java: ########## @@ -42,55 +41,51 @@ import org.apache.hadoop.io.compress.DefaultCodec; import org.apache.hadoop.io.compress.GzipCodec; import org.apache.hadoop.io.compress.zlib.BuiltInGzipDecompressor; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.experimental.categories.Category; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; /** * Along with CodecPool, this is copied from the class of the same name in hadoop-common. Modified * to accommodate changes to HBase's CodecPool. */ -@Category({ MiscTests.class, SmallTests.class }) +@Tag(MiscTests.TAG) +@Tag(SmallTests.TAG) public class TestCodecPool { - @ClassRule - public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestCodecPool.class); - private final String LEASE_COUNT_ERR = "Incorrect number of leased (de)compressors"; DefaultCodec codec; - @BeforeClass + @BeforeAll public static void beforeClass() { CodecPool.initLeaseCounting(); } - @Before + @BeforeEach public void setup() { this.codec = new DefaultCodec(); this.codec.setConf(new Configuration()); } - @Test(timeout = 10000) + @Test Review Comment: We remove 10s timeout here, need to add `@Timeout(value = 10, unit = TimeUnit.SECONDS)`? ########## hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java: ########## @@ -17,38 +17,36 @@ */ package org.apache.hadoop.hbase.types; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.math.BigDecimal; import java.util.Arrays; -import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.testclassification.MiscTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.util.Order; import org.apache.hadoop.hbase.util.PositionedByteRange; import org.apache.hadoop.hbase.util.SimplePositionedMutableByteRange; -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({ MiscTests.class, SmallTests.class }) +@Tag(MiscTests.TAG) +@Tag(SmallTests.TAG) public class TestStructNullExtension { - @ClassRule - public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestStructNullExtension.class); - /** * Verify null extension respects the type's isNullable field. */ - @Test(expected = NullPointerException.class) + @Test public void testNonNullableNullExtension() { - Struct s = new StructBuilder().add(new RawStringTerminated("|")) // not nullable - .toStruct(); - PositionedByteRange buf = new SimplePositionedMutableByteRange(4); - s.encode(buf, new Object[1]); + assertThrows(NullPointerException.class, () -> { + Struct s = new StructBuilder().add(new RawStringTerminated("|")) // not nullable Review Comment: ditto -- 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]
