Copilot commented on code in PR #8072:
URL: https://github.com/apache/hbase/pull/8072#discussion_r3070712199
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/compress/HFileTestBase.java:
##########
@@ -86,30 +86,30 @@ public void doTest(Configuration conf, Path path,
Compression.Algorithm compress
HFile.Reader reader = HFile.createReader(FS, path, cacheConf, true, conf);
try {
scanner = reader.getScanner(conf, false, false);
- assertTrue("Initial seekTo failed", scanner.seekTo());
+ assertTrue(scanner.seekTo(), "Initial seekTo failed");
Review Comment:
`scanner` is initialized to null and later closed unconditionally in the
`finally` block. If `HFile.createReader(...)` or `reader.getScanner(...)`
throws before `scanner` is assigned, this will throw a `NullPointerException`
and can mask the real failure. Consider using try-with-resources for the
reader/scanner (or at least null-checking `scanner`), and close the scanner
before closing the reader.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/compress/HFileTestBase.java:
##########
@@ -86,30 +86,30 @@ public void doTest(Configuration conf, Path path,
Compression.Algorithm compress
HFile.Reader reader = HFile.createReader(FS, path, cacheConf, true, conf);
try {
scanner = reader.getScanner(conf, false, false);
- assertTrue("Initial seekTo failed", scanner.seekTo());
+ assertTrue(scanner.seekTo(), "Initial seekTo failed");
do {
ExtendedCell kv = scanner.getCell();
- assertTrue("Read back an unexpected or invalid KV",
- testKvs.contains(KeyValueUtil.ensureKeyValue(kv)));
+ assertTrue(testKvs.contains(KeyValueUtil.ensureKeyValue(kv)),
+ "Read back an unexpected or invalid KV");
i++;
} while (scanner.next());
} finally {
reader.close();
scanner.close();
}
- assertEquals("Did not read back as many KVs as written", i,
testKvs.size());
+ assertEquals(i, testKvs.size(), "Did not read back as many KVs as
written");
// Test random seeks with pread
Random rand = ThreadLocalRandom.current();
LOG.info("Random seeking with " + fileContext);
reader = HFile.createReader(FS, path, cacheConf, true, conf);
try {
scanner = reader.getScanner(conf, false, true);
- assertTrue("Initial seekTo failed", scanner.seekTo());
+ assertTrue(scanner.seekTo(), "Initial seekTo failed");
for (i = 0; i < 100; i++) {
KeyValue kv = testKvs.get(rand.nextInt(testKvs.size()));
- assertEquals("Unable to find KV as expected: " + kv, 0,
scanner.seekTo(kv));
+ assertEquals(0, scanner.seekTo(kv), "Unable to find KV as expected: "
+ kv);
}
Review Comment:
In the second readback section, `scanner` is also closed unconditionally in
`finally` after being assigned inside the `try`. If `reader.getScanner(...)`
throws, `scanner` will still be null and `scanner.close()` will NPE. Use
try-with-resources or guard the close with a null check (and close the scanner
before the reader).
##########
hbase-compression/hbase-compression-zstd/src/test/java/org/apache/hadoop/hbase/io/compress/zstd/TestHFileCompressionZstd.java:
##########
@@ -86,7 +81,7 @@ public void testReconfLevels() throws Exception {
long len_2 = FS.getFileStatus(path_2).getLen();
LOG.info("Level 1 len {}", len_1);
LOG.info("Level 22 len {}", len_2);
- assertTrue("Reconfiguraton with ZSTD_LEVEL_KEY did not seem to work",
len_1 > len_2);
+ assertTrue(len_1 > len_2, "Reconfiguraton with ZSTD_LEVEL_KEY did not seem
to work");
Review Comment:
Typo in assertion message: "Reconfiguraton" should be "Reconfiguration".
```suggestion
assertTrue(len_1 > len_2, "Reconfiguration with ZSTD_LEVEL_KEY did not
seem to work");
```
--
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]
