HBASE-23645 Fixed remaining Checkstyle violations in hbase-common tests

Signed-off-by: Viraj Jasani <vjasani@apache.org>

hbase-checkstyle/src/main/resources/hbase/checkstyle-suppressions.xml

@@ -47,4 +47,6 @@
   <suppress checks="ImportOrder" message="Extra separation in import group before"/>
   <suppress checks="MethodLength" files="org.apache.hadoop.hbase.thrift.DemoClient.java"/>
   <suppress checks="LineLength" files="org.apache.hadoop.hbase.util.ZKDataMigrator.java"/>
+  <suppress checks="InnerAssignment" files="org.apache.hadoop.hbase.Waiter"/>
+  <suppress checks="EmptyBlock" files="org.apache.hadoop.hbase.TestTimeout"/>
 </suppressions>

hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java

@@ -30,7 +30,6 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
-import java.io.IOException;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
@@ -54,7 +53,7 @@ public class TestKeyValue {
   private static final Logger LOG = LoggerFactory.getLogger(TestKeyValue.class);
 
   @Test
-  public void testColumnCompare() throws Exception {
+  public void testColumnCompare() {
     final byte[] a = Bytes.toBytes("aaa");
     byte[] family1 = Bytes.toBytes("abc");
     byte[] qualifier1 = Bytes.toBytes("def");
@@ -74,11 +73,10 @@ public class TestKeyValue {
   }
 
   /**
-   * Test a corner case when the family qualifier is a prefix of the
+   * Test a corner case when the family qualifier is a prefix of the column qualifier.
-   *  column qualifier.
    */
   @Test
-  public void testColumnCompare_prefix() throws Exception {
+  public void testColumnCompare_prefix() {
     final byte[] a = Bytes.toBytes("aaa");
     byte[] family1 = Bytes.toBytes("abc");
     byte[] qualifier1 = Bytes.toBytes("def");
@@ -90,7 +88,7 @@ public class TestKeyValue {
   }
 
   @Test
-  public void testBasics() throws Exception {
+  public void testBasics() {
     LOG.info("LOWKEY: " + KeyValue.LOWESTKEY.toString());
     String name = "testBasics";
     check(Bytes.toBytes(name),
@@ -117,7 +115,7 @@ public class TestKeyValue {
   }
 
   @Test
-  public void testPlainCompare() throws Exception {
+  public void testPlainCompare() {
     final byte[] a = Bytes.toBytes("aaa");
     final byte[] b = Bytes.toBytes("bbb");
     final byte[] fam = Bytes.toBytes("col");
@@ -145,7 +143,7 @@ public class TestKeyValue {
   }
 
   @Test
-  public void testMoreComparisons() throws Exception {
+  public void testMoreComparisons() {
     long now = System.currentTimeMillis();
 
     // Meta compares
@@ -187,10 +185,9 @@ public class TestKeyValue {
   /**
    * Tests cases where rows keys have characters below the ','.
    * See HBASE-832
-   * @throws IOException
    */
   @Test
-  public void testKeyValueBorderCases() throws IOException {
+  public void testKeyValueBorderCases() {
     // % sorts before , so if we don't do special comparator, rowB would
     // come before rowA.
     KeyValue rowA = new KeyValue(Bytes.toBytes("testtable,www.hbase.org/,1234"),
@@ -204,7 +201,6 @@ public class TestKeyValue {
     rowB = new KeyValue(Bytes.toBytes("testtable,$www.hbase.org/,99999"),
         Bytes.toBytes("fam"), Bytes.toBytes(""), Long.MAX_VALUE, (byte[])null);
     assertTrue(CellComparatorImpl.META_COMPARATOR.compare(rowA, rowB) < 0);
-
   }
 
   private void metacomparisons(final CellComparatorImpl c) {
@@ -241,7 +237,7 @@ public class TestKeyValue {
   }
 
   @Test
-  public void testBinaryKeys() throws Exception {
+  public void testBinaryKeys() {
     Set<KeyValue> set = new TreeSet<>(CellComparatorImpl.COMPARATOR);
     final byte[] fam = Bytes.toBytes("col");
     final byte[] qf = Bytes.toBytes("umn");
@@ -260,7 +256,7 @@ public class TestKeyValue {
     int count = 0;
     try {
       for (KeyValue k : set) {
-        assertTrue(count++ == k.getTimestamp());
+        assertEquals(count++, k.getTimestamp());
       }
     } catch (java.lang.AssertionError e) {
       // Expected
@@ -272,7 +268,7 @@ public class TestKeyValue {
     Collections.addAll(set, keys);
     count = 0;
     for (KeyValue k : set) {
-      assertTrue(count++ == k.getTimestamp());
+      assertEquals(count++, k.getTimestamp());
     }
   }
 
@@ -281,7 +277,6 @@ public class TestKeyValue {
     // Test multiple KeyValues in a single blob.
 
     // TODO actually write this test!
-
   }
 
   private final byte[] rowA = Bytes.toBytes("rowA");
@@ -289,19 +284,15 @@ public class TestKeyValue {
 
   private final byte[] family = Bytes.toBytes("family");
   private final byte[] qualA = Bytes.toBytes("qfA");
-  private final byte[] qualB = Bytes.toBytes("qfB");
 
-  private void assertKVLess(CellComparator c,
+  private void assertKVLess(CellComparator c, KeyValue less, KeyValue greater) {
-                            KeyValue less,
-                            KeyValue greater) {
     int cmp = c.compare(less,greater);
     assertTrue(cmp < 0);
     cmp = c.compare(greater,less);
     assertTrue(cmp > 0);
   }
 
-  private void assertKVLessWithoutRow(CellComparator c, int common, KeyValue less,
+  private void assertKVLessWithoutRow(CellComparator c, KeyValue less, KeyValue greater) {
-      KeyValue greater) {
     int cmp = c.compare(less, greater);
     assertTrue(cmp < 0);
     cmp = c.compare(greater, less);
@@ -334,12 +325,12 @@ public class TestKeyValue {
     KeyValue kv1_0 = new KeyValue(row, fami1, qual0, ts, KeyValue.Type.Put);
 
     // 'fami:qf1' < 'fami:qf2'
-    assertKVLessWithoutRow(c, 0, kv0_1, kv0_2);
+    assertKVLessWithoutRow(c, kv0_1, kv0_2);
     // 'fami:qf1' < 'fami1:'
-    assertKVLessWithoutRow(c, 0, kv0_1, kv1_0);
+    assertKVLessWithoutRow(c, kv0_1, kv1_0);
 
     // Test comparison by skipping the same prefix bytes.
-    /***
+    /*
      * KeyValue Format and commonLength:
      * |_keyLen_|_valLen_|_rowLen_|_rowKey_|_famiLen_|_fami_|_Quali_|....
      * ------------------|-------commonLength--------|--------------
@@ -347,13 +338,13 @@ public class TestKeyValue {
     int commonLength = KeyValue.ROW_LENGTH_SIZE + KeyValue.FAMILY_LENGTH_SIZE
         + row.length;
     // 'fa:' < 'fami:'. They have commonPrefix + 2 same prefix bytes.
-    assertKVLessWithoutRow(c, commonLength + 2, kv_0, kv0_0);
+    assertKVLessWithoutRow(c, kv_0, kv0_0);
     // 'fami:' < 'fami:qf1'. They have commonPrefix + 4 same prefix bytes.
-    assertKVLessWithoutRow(c, commonLength + 4, kv0_0, kv0_1);
+    assertKVLessWithoutRow(c, kv0_0, kv0_1);
     // 'fami:qf1' < 'fami1:'. They have commonPrefix + 4 same prefix bytes.
-    assertKVLessWithoutRow(c, commonLength + 4, kv0_1, kv1_0);
+    assertKVLessWithoutRow(c, kv0_1, kv1_0);
     // 'fami:qf1' < 'fami:qf2'. They have commonPrefix + 6 same prefix bytes.
-    assertKVLessWithoutRow(c, commonLength + 6, kv0_1, kv0_2);
+    assertKVLessWithoutRow(c, kv0_1, kv0_2);
   }
 
   @Test
@@ -404,7 +395,7 @@ public class TestKeyValue {
   }
 
   @Test
-  public void testCreateKeyOnly() throws Exception {
+  public void testCreateKeyOnly() {
     long ts = 1;
     byte[] value = Bytes.toBytes("a real value");
     byte[] evalue = new byte[0]; // empty value
@@ -470,7 +461,8 @@ public class TestKeyValue {
     byte[] metaValue1 = Bytes.toBytes("metaValue1");
     byte[] metaValue2 = Bytes.toBytes("metaValue2");
     KeyValue kv = new KeyValue(row, cf, q, HConstants.LATEST_TIMESTAMP, value, new Tag[] {
-        new ArrayBackedTag((byte) 1, metaValue1), new ArrayBackedTag((byte) 2, metaValue2) });
+      new ArrayBackedTag((byte) 1, metaValue1), new ArrayBackedTag((byte) 2, metaValue2)
+    });
     assertTrue(kv.getTagsLength() > 0);
     assertTrue(Bytes.equals(kv.getRowArray(), kv.getRowOffset(), kv.getRowLength(), row, 0,
       row.length));
@@ -498,7 +490,7 @@ public class TestKeyValue {
     assertTrue(meta1Ok);
     assertTrue(meta2Ok);
     Iterator<Tag> tagItr = PrivateCellUtil.tagsIterator(kv);
-    //Iterator<Tag> tagItr = kv.tagsIterator();
+
     assertTrue(tagItr.hasNext());
     Tag next = tagItr.next();
     assertEquals(10, next.getValueLength());
@@ -572,7 +564,7 @@ public class TestKeyValue {
   }
 
   @Test
-  public void testEqualsAndHashCode() throws Exception {
+  public void testEqualsAndHashCode() {
     KeyValue kvA1 = new KeyValue(Bytes.toBytes("key"), Bytes.toBytes("cf"),
         Bytes.toBytes("qualA"), Bytes.toBytes("1"));
     KeyValue kvA2 = new KeyValue(Bytes.toBytes("key"), Bytes.toBytes("cf"),
@@ -587,7 +579,6 @@ public class TestKeyValue {
     assertNotEquals(kvA1, kvB);
     assertEquals(kvA1.hashCode(), kvA2.hashCode());
     assertNotEquals(kvA1.hashCode(), kvB.hashCode());
-
   }
 
   @Test
@@ -599,7 +590,8 @@ public class TestKeyValue {
             Bytes.toBytes("2")),
       new KeyValue(Bytes.toBytes("key"), Bytes.toBytes("cf"), Bytes.toBytes("qualA"),
             System.currentTimeMillis(), Bytes.toBytes("2"),
-            new Tag[] { new ArrayBackedTag((byte) 120, "tagA"),
+            new Tag[] {
+              new ArrayBackedTag((byte) 120, "tagA"),
               new ArrayBackedTag((byte) 121, Bytes.toBytes("tagB")) }),
       new KeyValue(Bytes.toBytes("key"), Bytes.toBytes("cf"), Bytes.toBytes("qualA"),
             System.currentTimeMillis(), Bytes.toBytes("2"),
@@ -628,7 +620,7 @@ public class TestKeyValue {
     // can't add to testCheckKeyValueBytesFailureCase because it
     // goes through the InputStream KeyValue API which can't produce a null buffer
     try {
-      KeyValue kv = new KeyValue(null, 0, 0);
+      new KeyValue(null, 0, 0);
     } catch (IllegalArgumentException iae) {
       assertEquals("Invalid to have null byte array in KeyValue.", iae.getMessage());
       return;
@@ -724,7 +716,8 @@ public class TestKeyValue {
       "Invalid tag length at position=28, tagLength=3",
       "Invalid tag length at position=34, tagLength=4",
       "Some redundant bytes in KeyValue's buffer, startOffset=41, endOffset=42", null, null,
-      null, };
+      null,
+    };
     assertEquals(inputs.length, outputs.length);
     assertEquals(withTagsInputs.length, withTagsOutputs.length);
 

hbase-common/src/test/java/org/apache/hadoop/hbase/TestTimeout.java

@@ -23,7 +23,7 @@ import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-@Category({SmallTests.class})
+@Category(SmallTests.class)
 public class TestTimeout {
 
   @ClassRule
@@ -39,7 +39,8 @@ public class TestTimeout {
    * Enable to check if timeout works.
    * Can't enable as it waits 30seconds and expected doesn't do Exception catching
    */
-    @Ignore @Test
+  @Ignore
+  @Test
   public void infiniteLoop() {
     // Launch a background non-daemon thread.
     Thread t = new Thread("HangingThread") {

hbase-common/src/test/java/org/apache/hadoop/hbase/Waiter.java

@@ -16,7 +16,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.hadoop.hbase;
 
 import static org.junit.Assert.fail;
@@ -34,7 +33,6 @@ import org.slf4j.LoggerFactory;
  */
 @InterfaceAudience.Private
 public final class Waiter {
-
   private static final Logger LOG = LoggerFactory.getLogger(Waiter.class);
 
   /**
@@ -87,14 +85,12 @@ public final class Waiter {
    */
   @InterfaceAudience.Private
   public interface Predicate<E extends Exception> {
-
     /**
      * Perform a predicate evaluation.
      * @return the boolean result of the evaluation.
-     * @throws Exception thrown if the predicate evaluation could not evaluate.
+     * @throws E thrown if the predicate evaluation could not evaluate.
      */
     boolean evaluate() throws E;
-
   }
 
   /**
@@ -102,14 +98,12 @@ public final class Waiter {
    */
   @InterfaceAudience.Private
   public interface ExplainingPredicate<E extends Exception> extends Predicate<E> {
-
     /**
      * Perform a predicate evaluation.
      *
      * @return explanation of failed state
      */
     String explainFailure() throws E;
-
   }
 
   /**
@@ -139,7 +133,7 @@ public final class Waiter {
    *         wait is interrupted otherwise <code>-1</code> when times out
    */
   public static <E extends Exception> long waitFor(Configuration conf, long timeout,
-      Predicate<E> predicate) throws E {
+      Predicate<E> predicate) {
     return waitFor(conf, timeout, 100, true, predicate);
   }
 
@@ -157,7 +151,7 @@ public final class Waiter {
    *         wait is interrupted otherwise <code>-1</code> when times out
    */
   public static <E extends Exception> long waitFor(Configuration conf, long timeout, long interval,
-      Predicate<E> predicate) throws E {
+      Predicate<E> predicate) {
     return waitFor(conf, timeout, interval, true, predicate);
   }
 
@@ -176,14 +170,14 @@ public final class Waiter {
    *         wait is interrupted otherwise <code>-1</code> when times out
    */
   public static <E extends Exception> long waitFor(Configuration conf, long timeout, long interval,
-      boolean failIfTimeout, Predicate<E> predicate) throws E {
+      boolean failIfTimeout, Predicate<E> predicate) {
     long started = System.currentTimeMillis();
     long adjustedTimeout = (long) (getWaitForRatio(conf) * timeout);
     long mustEnd = started + adjustedTimeout;
-    long remainderWait = 0;
+    long remainderWait;
-    long sleepInterval = 0;
+    long sleepInterval;
-    Boolean eval = false;
+    boolean eval;
-    Boolean interrupted = false;
+    boolean interrupted = false;
 
     try {
       LOG.info(MessageFormat.format("Waiting up to [{0}] milli-secs(wait.for.ratio=[{1}])",
@@ -192,7 +186,7 @@ public final class Waiter {
               && (remainderWait = mustEnd - System.currentTimeMillis()) > 0) {
         try {
           // handle tail case when remainder wait is less than one interval
-          sleepInterval = (remainderWait > interval) ? interval : remainderWait;
+          sleepInterval = Math.min(remainderWait, interval);
           Thread.sleep(sleepInterval);
         } catch (InterruptedException e) {
           eval = predicate.evaluate();
@@ -232,5 +226,4 @@ public final class Waiter {
       return "";
     }
   }
-
 }

hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestCopyOnWriteMaps.java

@@ -37,7 +37,6 @@ import org.junit.experimental.categories.Category;
 
 @Category({ MiscTests.class, SmallTests.class })
 public class TestCopyOnWriteMaps {
-
   @ClassRule
   public static final HBaseClassTestRule CLASS_RULE =
       HBaseClassTestRule.forClass(TestCopyOnWriteMaps.class);
@@ -62,12 +61,12 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testSize() throws Exception {
+  public void testSize() {
     assertEquals("Size should always be equal", m.size(), csm.size());
   }
 
   @Test
-  public void testIsEmpty() throws Exception {
+  public void testIsEmpty() {
     m.clear();
     assertTrue(m.isEmpty());
     m.put(100L, 100L);
@@ -77,7 +76,7 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testFindOnEmpty() throws Exception {
+  public void testFindOnEmpty() {
     m.clear();
     assertTrue(m.isEmpty());
     assertNull(m.get(100L));
@@ -85,10 +84,8 @@ public class TestCopyOnWriteMaps {
     assertEquals(0, m.tailMap(100L).entrySet().size());
   }
 
-
   @Test
-  public void testLowerKey() throws Exception {
+  public void testLowerKey() {
-
     assertEquals(csm.lowerKey(400L), m.lowerKey(400L));
     assertEquals(csm.lowerKey(-1L), m.lowerKey(-1L));
 
@@ -99,7 +96,7 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testFloorEntry() throws Exception {
+  public void testFloorEntry() {
     for (int i = 0 ; i < 100; i++) {
       Long key = ThreadLocalRandom.current().nextLong();
       assertEquals(csm.floorEntry(key), m.floorEntry(key));
@@ -107,7 +104,7 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testFloorKey() throws Exception {
+  public void testFloorKey() {
     for (int i = 0 ; i < 100; i++) {
       Long key = ThreadLocalRandom.current().nextLong();
       assertEquals(csm.floorKey(key), m.floorKey(key));
@@ -115,8 +112,7 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testCeilingKey() throws Exception {
+  public void testCeilingKey() {
-
     assertEquals(csm.ceilingKey(4000L), m.ceilingKey(4000L));
     assertEquals(csm.ceilingKey(400L), m.ceilingKey(400L));
     assertEquals(csm.ceilingKey(-1L), m.ceilingKey(-1L));
@@ -128,8 +124,7 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testHigherKey() throws Exception {
+  public void testHigherKey() {
-
     assertEquals(csm.higherKey(4000L), m.higherKey(4000L));
     assertEquals(csm.higherKey(400L), m.higherKey(400L));
     assertEquals(csm.higherKey(-1L), m.higherKey(-1L));
@@ -141,24 +136,24 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testRemove() throws Exception {
+  public void testRemove() {
     for (Map.Entry<Long, Long> e : csm.entrySet()) {
       assertEquals(csm.remove(e.getKey()), m.remove(e.getKey()));
-      assertEquals(null, m.remove(e.getKey()));
+      assertNull(m.remove(e.getKey()));
     }
   }
 
   @Test
-  public void testReplace() throws Exception {
+  public void testReplace() {
     for (Map.Entry<Long, Long> e : csm.entrySet()) {
       Long newValue = ThreadLocalRandom.current().nextLong();
       assertEquals(csm.replace(e.getKey(), newValue), m.replace(e.getKey(), newValue));
     }
-    assertEquals(null, m.replace(MAX_RAND + 100L, ThreadLocalRandom.current().nextLong()));
+    assertNull(m.replace(MAX_RAND + 100L, ThreadLocalRandom.current().nextLong()));
   }
 
   @Test
-  public void testReplace1() throws Exception {
+  public void testReplace1() {
     for (Map.Entry<Long, Long> e : csm.entrySet()) {
       Long newValue = ThreadLocalRandom.current().nextLong();
       assertEquals(csm.replace(e.getKey(), e.getValue() + 1, newValue),
@@ -168,22 +163,18 @@ public class TestCopyOnWriteMaps {
       assertEquals(newValue, m.get(e.getKey()));
       assertEquals(csm.get(e.getKey()), m.get(e.getKey()));
     }
-    assertEquals(null, m.replace(MAX_RAND + 100L, ThreadLocalRandom.current().nextLong()));
+    assertNull(m.replace(MAX_RAND + 100L, ThreadLocalRandom.current().nextLong()));
   }
 
   @Test
   public void testMultiAdd() throws InterruptedException {
-
     Thread[] threads = new Thread[10];
     for (int i = 0 ; i < threads.length; i++) {
-      threads[i] = new Thread(new Runnable() {
+      threads[i] = new Thread(() -> {
-        @Override
-        public void run() {
         for (int j = 0; j < 5000; j++) {
           m.put(ThreadLocalRandom.current().nextLong(),
               ThreadLocalRandom.current().nextLong());
         }
-        }
       });
     }
 
@@ -197,33 +188,32 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testFirstKey() throws Exception {
+  public void testFirstKey() {
     assertEquals(csm.firstKey(), m.firstKey());
   }
 
   @Test
-  public void testLastKey() throws Exception {
+  public void testLastKey() {
     assertEquals(csm.lastKey(), m.lastKey());
   }
 
   @Test
-  public void testFirstEntry() throws Exception {
+  public void testFirstEntry() {
     assertEquals(csm.firstEntry().getKey(), m.firstEntry().getKey());
     assertEquals(csm.firstEntry().getValue(), m.firstEntry().getValue());
     assertEquals(csm.firstEntry(), m.firstEntry());
   }
 
   @Test
-  public void testLastEntry() throws Exception {
+  public void testLastEntry() {
     assertEquals(csm.lastEntry().getKey(), m.lastEntry().getKey());
     assertEquals(csm.lastEntry().getValue(), m.lastEntry().getValue());
     assertEquals(csm.lastEntry(), m.lastEntry());
   }
 
   @Test
-  public void testKeys() throws Exception {
+  public void testKeys() {
     for (Long key : csm.keySet()) {
-      //assertTrue(m.containsKey(key));
       assertNotNull(m.get(key));
       assertNotNull(m.remove(key));
       assertNull(m.get(key));
@@ -231,15 +221,15 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testValues() throws Exception {
+  public void testValues() {
     for (Long value : m.values()) {
-      assertTrue(csm.values().contains(value));
+      assertTrue(csm.containsValue(value));
       assertTrue(m.containsValue(value));
     }
   }
 
   @Test
-  public void testTailMap() throws Exception {
+  public void testTailMap() {
     Map<Long, Long> fromCsm = csm.tailMap(50L);
     Map<Long, Long> fromM = m.tailMap(50L);
     assertEquals(fromCsm, fromM);
@@ -254,7 +244,7 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testTailMapExclusive() throws Exception {
+  public void testTailMapExclusive() {
     m.clear();
     m.put(100L, 100L);
     m.put(101L, 101L);
@@ -273,7 +263,7 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testTailMapInclusive() throws Exception {
+  public void testTailMapInclusive() {
     m.clear();
     m.put(100L, 100L);
     m.put(101L, 101L);
@@ -299,7 +289,7 @@ public class TestCopyOnWriteMaps {
   }
 
   @Test
-  public void testPut() throws Exception {
+  public void testPut() {
     m.clear();
     m.put(100L, 100L);
     m.put(101L, 101L);