Copilot commented on code in PR #8274:
URL: https://github.com/apache/hbase/pull/8274#discussion_r3298348383
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionTracing.java:
##########
@@ -102,7 +97,7 @@ public void setUp() throws IOException {
region = UTIL.createLocalHRegion(info, desc);
Review Comment:
In setUp(), you create a WAL + region via HRegion.createHRegion(..., wal)
and then immediately overwrite `region` with `UTIL.createLocalHRegion(info,
desc)`. This leaks the first region instance, and also leaves `wal`
disconnected from the `region` field being tested/closed. Please create the
region only once (either use `UTIL.createLocalHRegion(info, conf, desc, wal)`
or let HTU create/close the WAL via `createLocalHRegion` + `closeRegionAndWAL`).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java:
##########
@@ -593,13 +590,13 @@ private void verifyAfterTtl(HRegion region, long ts)
throws IOException {
get.readAllVersions();
get.setTimestamp(ts - 3);
result = region.get(get);
- Assert.assertEquals(result.getColumnCells(c0, c0).size(), 0);
+ assertEquals(result.getColumnCells(c0, c0).size(), 0);
get = new Get(T1);
get.readAllVersions();
get.setTimeRange(0, ts - 2);
result = region.get(get);
- Assert.assertEquals(result.getColumnCells(c0, c0).size(), 0);
+ assertEquals(result.getColumnCells(c0, c0).size(), 0);
}
Review Comment:
The assertEquals argument order is reversed here (expected should be 0,
actual should be the size). Reordering improves failure diagnostics and matches
the convention used elsewhere in this PR.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStoreFile.java:
##########
@@ -805,7 +797,7 @@ public void testReseek() throws Exception {
StoreFileScanner s = getStoreFileScanner(reader, false, false);
s.reseek(k);
- assertNotNull("Intial reseek should position at the beginning of the
file", s.peek());
+ assertNotNull(s.peek(), "Intial reseek should position at the beginning of
the file");
Review Comment:
Typo in assertion message: "Intial reseek" should be "Initial reseek".
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java:
##########
@@ -593,13 +590,13 @@ private void verifyAfterTtl(HRegion region, long ts)
throws IOException {
get.readAllVersions();
get.setTimestamp(ts - 3);
result = region.get(get);
- Assert.assertEquals(result.getColumnCells(c0, c0).size(), 0);
+ assertEquals(result.getColumnCells(c0, c0).size(), 0);
Review Comment:
The assertEquals argument order is reversed here (expected should be 0,
actual should be the size). Reordering improves failure diagnostics and matches
the convention used elsewhere in this PR.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableMetricsMap.java:
##########
@@ -68,14 +63,14 @@ public void testMetricsMap() throws InterruptedException {
rsm.updateCompaction(tableName, true, 100, 200, 300, 400, 500);
int metricsMapSize = agg.getMetricsRegistry().getMetricsMap().size();
- assertTrue("table metrics added then metricsMapSize should larger than 0",
metricsMapSize > 0);
+ assertTrue(metricsMapSize > 0, "table metrics added then metricsMapSize
should larger than 0");
Review Comment:
Grammar in the assertion message: "should larger than 0" should be "should
be larger than 0" (or similar) to read correctly.
--
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]
