Copilot commented on code in PR #8072:
URL: https://github.com/apache/hbase/pull/8072#discussion_r3149598273
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/compress/HFileTestBase.java:
##########
@@ -82,38 +82,29 @@ public void doTest(Configuration conf, Path path,
Compression.Algorithm compress
// read it back in
LOG.info("Reading with " + fileContext);
int i = 0;
- HFileScanner scanner = null;
- HFile.Reader reader = HFile.createReader(FS, path, cacheConf, true, conf);
- try {
- scanner = reader.getScanner(conf, false, false);
- assertTrue("Initial seekTo failed", scanner.seekTo());
+ try (HFile.Reader reader = HFile.createReader(FS, path, cacheConf, true,
conf);
+ HFileScanner scanner = reader.getScanner(conf, false, false)) {
+ 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");
Review Comment:
`assertEquals` in JUnit Jupiter expects arguments in the order (expected,
actual, message). Here it is currently `assertEquals(i, testKvs.size(), ...)`,
which will invert expected/actual in failure output. Swap the first two
arguments so the expected value is `testKvs.size()` and the actual value is `i`.
##########
hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseParameterizedTemplateProvider.java:
##########
@@ -40,9 +41,11 @@
* <p>
* When you want to use this provider, annotation the test class with
* {@link HBaseParameterizedTestTemplate}, and provide a static method named
"parameters" for
- * providing the arguments. The method must have no parameter, and return a
Stream<Arguments>.
- * All the test method should be marked with {@link
org.junit.jupiter.api.TestTemplate}, not
- * {@link org.junit.jupiter.api.Test} or {@link
org.junit.jupiter.params.ParameterizedTest}.
+ * providing the arguments if you want to pass parameters to the constructor.
The method must have
+ * no parameter, and return a Stream<Arguments>. For test classes with
no constructor
+ * parameters, omitting the method means running a single invocation. All the
test method should be
+ * marked with {@link org.junit.jupiter.api.TestTemplate}, not {@link
org.junit.jupiter.api.Test} or
+ * {@link org.junit.jupiter.params.ParameterizedTest}.
Review Comment:
Javadoc grammar: "annotation the test class" should be "annotate the test
class" (and similarly "All the test method" -> "All test methods") to keep the
documentation clear and professional.
--
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]
