HBASE-30067 Upgrade hbase-server to use junit5 Part10#8046
Conversation
liuxiaocs7
commented
Apr 9, 2026
- see: HBASE-30067
There was a problem hiding this comment.
Pull request overview
Migrates a set of hbase-server tests (snapshot, mob, ipc) from JUnit4 to JUnit5 as part of HBASE-30067.
Changes:
- Replaces JUnit4 annotations/assertions/rules (
@BeforeClass,@Category,@Rule,Assert.*, parameterized runner) with JUnit Jupiter equivalents (@BeforeAll,@Tag,TestInfo,Assertions.*,@TestTemplate+@HBaseParameterizedTestTemplate). - Updates several tests to generate per-test names via
TestInfoinstead of JUnit4TestName/TableNameTestRule. - Adjusts parameterized tests to use HBase’s JUnit5 template provider and
Stream<Arguments>parameter sources.
Reviewed changes
Copilot reviewed 60 out of 60 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotWhenChoreCleaning.java | JUnit5 migration for snapshot-cleaner concurrency test. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotStoreFileSize.java | JUnit5 migration for snapshot storefile size validation. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotManifest.java | JUnit5 migration for snapshot manifest unit tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java | JUnit5 migration for snapshot description utils tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotClientRetries.java | JUnit5 migration; replaces expected-exception with assertThrows and uses TestInfo for table naming. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestRestoreSnapshotHelper.java | JUnit5 migration for restore/clone snapshot helper tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestRestoreFlushSnapshotFromClient.java | JUnit5 migration for restore-from-client integration tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestRegionSnapshotTask.java | JUnit5 migration for region snapshot task integration test. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestMobRestoreSnapshotHelper.java | JUnit5 migration for MOB restore snapshot helper subclass. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestMobRestoreFlushSnapshotFromClient.java | JUnit5 migration for MOB restore/clone-from-client tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestMobFlushSnapshotFromClient.java | JUnit5 migration for MOB flush snapshot client tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestFlushSnapshotFromClient.java | JUnit5 migration for base flush snapshot-from-client test suite. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestConcurrentFlushSnapshotFromClient.java | JUnit5 migration for concurrent snapshot client tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/SnapshotTestingUtils.java | JUnit5 assertion API updates for snapshot test helper utilities. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/MobSnapshotTestingUtils.java | JUnit5 assertion API updates for MOB snapshot helpers. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestRSMobFileCleanerChore.java | JUnit5 migration for RS mob cleaner chore tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobWithByteBuffAllocator.java | JUnit5 migration for MOB ByteBuff allocator test. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobUtils.java | JUnit5 migration; changes helper signature to accept method-name string. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobStoreScanner.java | JUnit5 migration; replaces TestName rule with TestInfo. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobStoreCompaction.java | Converts JUnit4 parameterized to JUnit5 template provider + Jupiter assertions. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobFileName.java | JUnit5 migration for mob filename unit test. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobFileLink.java | JUnit5 migration; uses TestInfo instead of TestName. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobFileCleanupUtil.java | JUnit5 migration for MOB file cleanup util tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobFileCache.java | JUnit5 migration for MOB file cache tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobFile.java | JUnit5 migration; uses TestInfo for case naming. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobDataBlockEncoding.java | JUnit5 migration for MOB data block encoding test. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobCompactionWithException.java | JUnit5 migration; replaces TestName with TestInfo for unique naming. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobCompactionWithDefaults.java | Converts JUnit4 parameterized to JUnit5 template provider; updates naming + assertions. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobCompactionRegularRegionBatchMode.java | JUnit5 migration; parameterized template usage and lifecycle updates. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobCompactionOptRegionBatchMode.java | JUnit5 migration; parameterized template usage and lifecycle updates. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobCompactionOptMode.java | JUnit5 migration; parameterized template usage and tag updates. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestExpiredMobFileCleanerChore.java | JUnit5 migration for master-side expired MOB file cleaner chore tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestExpiredMobFileCleaner.java | JUnit5 migration for expired MOB file cleaner tool tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestDefaultMobStoreFlusher.java | Converts JUnit4 parameterized to JUnit5 template provider; updates assertions and naming. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestCachedMobFile.java | JUnit5 migration; replaces TestName + Assert with Jupiter equivalents. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/mob/MobTestUtil.java | JUnit5 assertion API updates for MOB test utility methods. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestSimpleRpcServer.java | JUnit5 migration; uses TestInfo for per-test TableName. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestSimpleRpcScheduler.java | JUnit5 migration; replaces TestName and updates assertions. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestServerCall.java | JUnit5 migration; assertion argument order and message updates. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestSecurityRpcSentBytesMetrics.java | JUnit5 migration for secure RPC metrics test setup/teardown. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestSecureSimpleRpcServer.java | JUnit5 migration for secure simple RPC server test class. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestSecureNettyRpcServer.java | Converts to JUnit5 template provider + Jupiter lifecycle; integrates Kerberos setup. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestRWQueueRpcExecutor.java | JUnit5 migration; replaces TestName and updates assertions/messages. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestRpcSkipInitialSaslHandshake.java | JUnit5 migration for SASL-handshake skipping behavior test. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestRpcServerTraceLogging.java | JUnit5 migration for trace logging configuration test. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestRpcServerSlowConnectionSetup.java | Converts JUnit4 parameterized to JUnit5 template provider for rpc server impl variants. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestRpcMetrics.java | JUnit5 migration for rpc metrics unit tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestRpcHandlerException.java | Converts JUnit4 parameterized to JUnit5 template provider for handler exception tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestRpcConnectionHeader.java | JUnit5 migration; removes JUnit4 rules/classrule usage. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestRpcClientLeaks.java | JUnit5 migration; uses TestInfo for per-test table names. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestProtoBufRpc.java | Converts JUnit4 parameterized + expected-exception to JUnit5 template + assertThrows. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestNettyRpcServer.java | Converts JUnit4 parameterized to JUnit5 template provider; per-invocation allocator variants. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestNettyIPCCloseConnection.java | JUnit5 migration for netty channel close behavior test. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestNettyChannelWritability.java | JUnit5 migration; switches assertions to Jupiter. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestMultipleServerPrincipalsIPC.java | Converts JUnit4 parameterized to JUnit5 template provider for rpc server/client combinations. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestMasterFifoRpcScheduler.java | JUnit5 migration for master FIFO rpc scheduler tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestHBaseClient.java | JUnit5 migration; uses Jupiter assertions. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestFifoRpcScheduler.java | JUnit5 migration for FIFO rpc scheduler unit tests. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestCallRunner.java | JUnit5 migration; switches OpenTelemetry testing from JUnit4 rule to JUnit5 extension. |
| hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestBufferChain.java | JUnit5 migration for BufferChain unit tests. |
Comments suppressed due to low confidence (1)
hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotWhenChoreCleaning.java:62
- The logger is initialized with TestSnapshotClientRetries.class, which is unrelated to this test class and will mislabel log output. Please use TestSnapshotWhenChoreCleaning.class (or getClass() for non-static) when creating the logger.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 64 out of 64 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobStoreCompaction.java:156
- tearDown() deletes
UTIL.getDataTestDir(), but init() created the region under a different HBaseTestingUtil instance (a locally scopedUTIL). If these resolve to different paths, this can leak test files (or delete an unexpected directory). Suggest ensuring the same HBaseTestingUtil/dataTestDir instance used during createRegionAndWAL is also used for cleanup.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @BeforeEach | ||
| public void setUp(TestInfo testInfo) { | ||
| testMethodName = testInfo.getTestMethod().get().getName(); | ||
| } |
There was a problem hiding this comment.
In this parameterized test (via @HBaseParameterizedTestTemplate), deriving the table/test identifier from only testInfo.getTestMethod().get().getName() means both parameter invocations will reuse the same name. This can cause collisions between iterations (e.g., leftover FS/table artifacts if a previous iteration fails), since JUnit4 parameterized method names previously included a "[index]" suffix. Consider incorporating the parameterized display name (e.g., testInfo.getDisplayName()) or the parameter value/index into the generated name. See the existing pattern in BasicReadWriteWithDifferentConnectionRegistriesTestBase.java:111-115 which mixes getDisplayName() into the table name.
There was a problem hiding this comment.
testInfo.getTestMethod().get().getName() + testInfo.getDisplayName() returns testSmallerValue1: useFileBasedSFT=false
use testInfo.getTestMethod().get().getName() + testInfo.getDisplayName().replaceAll("[:= ]", "_").replaceAll("_+", "_").trim(), returns testSmallerValue1_useFileBasedSFT_false
| { SimpleByteBufAllocator.class.getName() } }); | ||
| @BeforeEach | ||
| public void setUpTable(TestInfo testInfo) { | ||
| tableName = TableName.valueOf(testInfo.getTestMethod().get().getName()); |
There was a problem hiding this comment.
This test is parameterized via @HBaseParameterizedTestTemplate(name = "{index}: allocatorType={0}"), but tableName is derived from only the method name, so all allocatorType iterations share the same table name. If the mini cluster/root dir is reused across iterations, this increases the chance of name collisions and flakiness. Consider incorporating testInfo.getDisplayName() (contains the index/param) or allocatorType into the table name, consistent with BasicReadWriteWithDifferentConnectionRegistriesTestBase.java:111-115.
| tableName = TableName.valueOf(testInfo.getTestMethod().get().getName()); | |
| String sanitizedAllocatorType = allocatorType.replaceAll("[^a-zA-Z0-9_.-]", "_"); | |
| tableName = TableName | |
| .valueOf(testInfo.getTestMethod().get().getName() + "_" + sanitizedAllocatorType); |
| this.conf = conf; | ||
| this.mobCellThreshold = mobThreshold; | ||
|
|
||
| HBaseTestingUtil UTIL = new HBaseTestingUtil(conf); | ||
|
|
||
| compactionThreshold = conf.getInt("hbase.hstore.compactionThreshold", 3); | ||
| familyDescriptor = ColumnFamilyDescriptorBuilder.newBuilder(COLUMN_FAMILY).setMobEnabled(true) |
There was a problem hiding this comment.
In init(), the local variable HBaseTestingUtil UTIL = new HBaseTestingUtil(conf); shadows the class-level static UTIL. Later code mixes both (local UTIL for createRegionAndWAL, but class-level UTIL for configuration elsewhere), which is easy to misuse and makes it unclear which dataTestDir is being used. Consider removing the shadowing local variable or renaming/storing the instance used for getDataTestDir so setup/cleanup operate on the same paths.
|
I have checked that the number of unit tests was consistent before and after, and that the unit tests were run in the same way, so let me merge it first! |