Copilot commented on code in PR #8283:
URL: https://github.com/apache/hbase/pull/8283#discussion_r3322200809
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBytesReadServerSideScanMetrics.java:
##########
@@ -759,7 +759,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:
The assertion uses `assertEquals(actual, expected)` which works but produces
confusing failure output. JUnit expects `assertEquals(expected, actual)` (and
the comment has a typo).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java:
##########
@@ -744,7 +738,7 @@ public List<Path> compact(ThroughputController
throughputController, User user)
this.wait();
}
} catch (InterruptedException e) {
- Assume.assumeNoException(e);
+ Assumptions.abort(e.getMessage());
}
Review Comment:
The `InterruptedException` path should restore the thread interrupt flag.
Otherwise later code (or the test framework) may miss the interrupt and
continue unexpectedly.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBulkLoad.java:
##########
@@ -77,19 +74,19 @@ public Object answer(InvocationOnMock invocation) {
walKey.setWriteEntry(we);
}
return 01L;
Review Comment:
Avoid integer literals with a leading zero (`01L`) as they are easy to
misread (and are octal in Java).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBytesReadServerSideScanMetrics.java:
##########
@@ -808,24 +808,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` argument order is reversed here. Using
`assertEquals(expected, actual)` will make failures easier to understand
(expected value comes from the trailer).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBulkLoad.java:
##########
@@ -102,13 +99,13 @@ public Object answer(InvocationOnMock invocation) {
walKey.setWriteEntry(we);
}
return 01L;
Review Comment:
Avoid integer literals with a leading zero (`01L`) as they are easy to
misread (and are octal in Java).
--
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]
