Copilot commented on code in PR #8185:
URL: https://github.com/apache/hbase/pull/8185#discussion_r3177878229
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactorMemLeak.java:
##########
@@ -89,16 +88,16 @@ public static void tearDown() throws Exception {
private void assertMajorCompactionOK(TableName tableName) {
List<HRegion> regions =
UTIL.getHBaseCluster().getRegionServerThreads().get(0).getRegionServer()
.getRegions(tableName);
- Assert.assertEquals(regions.size(), 1);
+ assertEquals(regions.size(), 1);
HRegion region = regions.get(0);
- Assert.assertEquals(region.getStores().size(), 1);
+ assertEquals(region.getStores().size(), 1);
HStore store = region.getStore(FAMILY);
- Assert.assertEquals(store.getStorefilesCount(), 1);
+ assertEquals(store.getStorefilesCount(), 1);
Review Comment:
The assertEquals argument order is reversed here (expected should be the
constant, actual should be the computed value). Keeping the current order still
works when the values are equal, but produces confusing expected/actual
reporting on failure; prefer assertEquals(1, regions.size()), assertEquals(1,
region.getStores().size()), etc.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBytesReadServerSideScanMetrics.java:
##########
@@ -810,24 +809,24 @@ private void readHFile(HFile.Reader hfileReader,
CompoundBloomFilter cbf, KeyVal
block.release();
blockIter.freeBlocks();
- Assert.assertEquals(blockLevelsRead, trailer.getNumDataIndexLevels() + 1);
+ assertEquals(blockLevelsRead, trailer.getNumDataIndexLevels() + 1);
Review Comment:
assertEquals arguments are swapped here (expected should be
trailer.getNumDataIndexLevels() + 1, actual should be blockLevelsRead).
Swapping them will make expected/actual reporting correct if the assertion ever
fails.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBytesReadServerSideScanMetrics.java:
##########
@@ -761,7 +760,7 @@ private void readHFile(HFile.Reader hfileReader,
CompoundBloomFilter cbf, KeyVal
bloomBlock.getUncompressedSizeWithoutHeader(), cbf.getHash(),
cbf.getHashCount());
bytesReadFunction.accept(bloomBlock);
// Asser that the block read is a bloom block
- Assert.assertEquals(bloomBlock.getBlockType(), BlockType.BLOOM_CHUNK);
+ assertEquals(bloomBlock.getBlockType(), BlockType.BLOOM_CHUNK);
Review Comment:
assertEquals arguments are swapped here (expected should be
BlockType.BLOOM_CHUNK, actual should be bloomBlock.getBlockType()). This only
affects failure diagnostics, but makes assertion output much clearer when it
fails.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCleanupMetaWAL.java:
##########
@@ -31,30 +30,25 @@
import org.apache.hadoop.hbase.master.procedure.ServerCrashProcedure;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.util.CommonFSUtils;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
-import org.junit.Test;
-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.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-@Category(MediumTests.class)
+@Tag(MediumTests.TAG)
public class TestCleanupMetaWAL {
private static final Logger LOG =
LoggerFactory.getLogger(TestCleanupMetaWAL.class);
private final static HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestCleanupMetaWAL.class);
-
- @BeforeClass
+ @BeforeAll
public static void before() throws Exception {
TEST_UTIL.startMiniCluster(2);
}
- @AfterClass
+ @AfterAll
public static void after() throws Exception {
TEST_UTIL.shutdownMiniZKCluster();
Review Comment:
The test starts a full mini cluster in before(), but after() only calls
shutdownMiniZKCluster(). This leaves the HBase mini cluster (and potentially
DFS) running and can leak threads/resources into subsequent tests. Call
shutdownMiniCluster() (or at least shutdownMiniHBaseCluster +
shutdownMiniDFSCluster + shutdownMiniZKCluster) to match startMiniCluster().
--
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]
