hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From md...@apache.org
Subject [2/2] hbase git commit: HBASE-18656 First issues found by error-prone
Date Thu, 24 Aug 2017 17:17:09 GMT
HBASE-18656 First issues found by error-prone


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/bd0b0afa
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/bd0b0afa
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/bd0b0afa

Branch: refs/heads/master
Commit: bd0b0afa61e9b0bc54316596dd9ae6bd8a2fe684
Parents: 1b4e935
Author: Mike Drob <mdrob@apache.org>
Authored: Wed Aug 23 16:43:50 2017 -0500
Committer: Mike Drob <mdrob@apache.org>
Committed: Thu Aug 24 12:16:31 2017 -0500

----------------------------------------------------------------------
 .../hadoop/hbase/util/ConcatenatedLists.java    | 76 +-------------------
 .../apache/hadoop/hbase/TestChoreService.java   | 14 ++--
 .../hbase/util/TestConcatenatedLists.java       |  4 +-
 .../hadoop/hbase/util/TestDrainBarrier.java     |  6 +-
 4 files changed, 17 insertions(+), 83 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/bd0b0afa/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java
b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java
index ba54f9d..4217906 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java
@@ -18,9 +18,8 @@
  */
 package org.apache.hadoop.hbase.util;
 
-import java.lang.reflect.Array;
+import java.util.AbstractCollection;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.NoSuchElementException;
 
@@ -33,7 +32,7 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience;
  * NOTE: Doesn't implement list as it is not necessary for current usage, feel free to add.
  */
 @InterfaceAudience.Private
-public class ConcatenatedLists<T> implements Collection<T> {
+public class ConcatenatedLists<T> extends AbstractCollection<T> {
   protected final ArrayList<List<T>> components = new ArrayList<>();
   protected int size = 0;
 
@@ -56,77 +55,6 @@ public class ConcatenatedLists<T> implements Collection<T>
{
   }
 
   @Override
-  public boolean isEmpty() {
-    return this.size == 0;
-  }
-
-  @Override
-  public boolean contains(Object o) {
-    for (List<T> component : this.components) {
-      if (component.contains(o)) return true;
-    }
-    return false;
-  }
-
-  @Override
-  public boolean containsAll(Collection<?> c) {
-    for (Object o : c) {
-      if (!contains(o)) return false;
-    }
-    return true;
-  }
-
-  @Override
-  public Object[] toArray() {
-    return toArray((Object[])Array.newInstance(Object.class, this.size));
-  }
-
-  @Override
-  @SuppressWarnings("unchecked")
-  public <U> U[] toArray(U[] a) {
-    U[] result = (a.length == this.size()) ? a
-        : (U[])Array.newInstance(a.getClass().getComponentType(), this.size);
-    int i = 0;
-    for (List<T> component : this.components) {
-      for (T t : component) {
-        result[i] = (U)t;
-        ++i;
-      }
-    }
-    return result;
-  }
-
-  @Override
-  public boolean add(T e) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public boolean remove(Object o) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public boolean addAll(Collection<? extends T> c) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public boolean removeAll(Collection<?> c) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public boolean retainAll(Collection<?> c) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public void clear() {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
   public java.util.Iterator<T> iterator() {
     return new Iterator();
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/bd0b0afa/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
index f290e1d..06ce6d0 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
@@ -24,6 +24,8 @@ import static org.junit.Assert.assertTrue;
 
 import java.util.concurrent.TimeUnit;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.hbase.TestChoreService.ScheduledChoreSamples.CountingChore;
 import org.apache.hadoop.hbase.TestChoreService.ScheduledChoreSamples.DoNothingChore;
 import org.apache.hadoop.hbase.TestChoreService.ScheduledChoreSamples.FailInitialChore;
@@ -36,6 +38,8 @@ import org.junit.experimental.categories.Category;
 
 @Category(SmallTests.class)
 public class TestChoreService {
+  public static final Log log = LogFactory.getLog(TestChoreService.class);
+
   /**
    * A few ScheduledChore samples that are useful for testing with ChoreService
    */
@@ -75,7 +79,7 @@ public class TestChoreService {
         try {
           Thread.sleep(getPeriod() * 2);
         } catch (InterruptedException e) {
-          e.printStackTrace();
+          log.warn("", e);
         }
         return true;
       }
@@ -85,7 +89,7 @@ public class TestChoreService {
         try {
           Thread.sleep(getPeriod() * 2);
         } catch (InterruptedException e) {
-          //e.printStackTrace();
+          log.warn("", e);
         }
       }
     }
@@ -126,7 +130,7 @@ public class TestChoreService {
         try {
           Thread.sleep(sleepTime);
         } catch (InterruptedException e) {
-          e.printStackTrace();
+          log.warn("", e);
         }
         return true;
       }
@@ -136,7 +140,7 @@ public class TestChoreService {
         try {
           Thread.sleep(sleepTime);
         } catch (Exception e) {
-          System.err.println(e.getStackTrace());
+          log.warn("", e);
         }
       }
     }
@@ -174,7 +178,7 @@ public class TestChoreService {
       }
 
       private void outputTickCount() {
-        System.out.println("Chore: " + getName() + ". Count of chore calls: " + countOfChoreCalls);
+        log.info("Chore: " + getName() + ". Count of chore calls: " + countOfChoreCalls);
       }
 
       public int getCountOfChoreCalls() {

http://git-wip-us.apache.org/repos/asf/hbase/blob/bd0b0afa/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java
b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java
index cfd288d..08d5569 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java
@@ -67,7 +67,7 @@ public class TestConcatenatedLists {
     } catch (UnsupportedOperationException ex) {
     }
     try {
-      c.retainAll(Arrays.asList(0L, 1L));
+      c.retainAll(Arrays.asList(0L, 2L));
       fail("Should throw");
     } catch (UnsupportedOperationException ex) {
     }
@@ -118,9 +118,11 @@ public class TestConcatenatedLists {
     verify(c, 7);
   }
 
+  @SuppressWarnings("ModifyingCollectionWithItself")
   private void verify(ConcatenatedLists<Long> c, int last) {
     assertEquals((last == -1), c.isEmpty());
     assertEquals(last + 1, c.size());
+    // This check is O(n^2), test with care
     assertTrue(c.containsAll(c));
     Long[] array = c.toArray(new Long[c.size()]);
     List<Long> all = new ArrayList<>();

http://git-wip-us.apache.org/repos/asf/hbase/blob/bd0b0afa/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java
b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java
index 4542cbd..b5fa185 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java
@@ -46,7 +46,7 @@ public class TestDrainBarrier {
     DrainBarrier barrier = new DrainBarrier();
     try {
       barrier.endOp();
-      fail("Should have asserted");
+      throw new Error("Should have asserted");
     } catch (AssertionError e) {
     }
 
@@ -56,7 +56,7 @@ public class TestDrainBarrier {
     barrier.endOp();
     try {
       barrier.endOp();
-      fail("Should have asserted");
+      throw new Error("Should have asserted");
     } catch (AssertionError e) {
     }
   }
@@ -108,7 +108,7 @@ public class TestDrainBarrier {
     barrier.stopAndDrainOpsOnce();
     try {
       barrier.stopAndDrainOpsOnce();
-      fail("Should have asserted");
+      throw new Error("Should have asserted");
     } catch (AssertionError e) {
     }
   }


Mime
View raw message