[jira] [Commented] (HBASE-18119) Improve HFile readability and modify ChecksumUtil log level

[Zach York (JIRA)]

    [ 
https://issues.apache.org/jira/browse/HBASE-18119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16056576#comment-16056576
 ] 

Zach York commented on HBASE-18119:
-----------------------------------

{quote} 
+  @Test
+  public void testCheckHFileVersionForWrongConfiged() {
+    Configuration conf = TEST_UTIL.getConfiguration();
+    int version = conf.getInt(HFile.FORMAT_VERSION_KEY, 
HFile.MAX_FORMAT_VERSION);
+    conf.setInt(HFile.FORMAT_VERSION_KEY, HFile.MIN_FORMAT_VERSION);
+    try {
+      HFile.checkHFileVersion(conf);
+      fail();
+    } catch (IllegalArgumentException e) {
+      conf.setInt(HFile.FORMAT_VERSION_KEY, version);
+      
assertTrue(String.valueOf(version).equals(conf.get(HFile.FORMAT_VERSION_KEY)));
+    }
+  }
{quote}

I think we can simplify this test case to only include the failure case as your 
test above this should test this functionality.


{code:java}
  @Test(expected=IllegalArgumentException.class)
  public void testCheckHFileVersionNotEqualToMaxVersion() {
    Configuration conf = TEST_UTIL.getConfiguration();
    conf.setInt(HFile.FORMAT_VERSION_KEY, HFile.MIN_FORMAT_VERSION);
    HFile.checkHFileVersion(conf);
  }
{code}


Also:
bq. +  public void testCheckHFileVersionForCorrectConfiged(){
ForCorrectConfiged -> ForCorrectConfiguration Or even something like 
testCheckHFileVersionEqualToMaxVersion

Also please include a space between '()' and '{' 

> Improve HFile readability and modify ChecksumUtil log level
> -----------------------------------------------------------
>
>                 Key: HBASE-18119
>                 URL: https://issues.apache.org/jira/browse/HBASE-18119
>             Project: HBase
>          Issue Type: Improvement
>    Affects Versions: 2.0.0
>            Reporter: Qilin Cao
>            Assignee: Qilin Cao
>            Priority: Minor
>         Attachments: HBASE-18119-v1.patch, HBASE-18119-v2.patch, 
> HBASE-18119-v3.patch
>
>
> It is confused when I read the HFile.checkHFileVersion method, so I change 
> the if expression. Simultaneously, I change ChecksumUtil the info log level 
> to trace.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)