Skip to content

Commit

Permalink
HBASE-23621 Reduced the number of Checkstyle violations in tests of h…
Browse files Browse the repository at this point in the history 
…base-common

Signed-off-by: stack <[email protected]>
Signed-off-by: Viraj Jasani <[email protected]>
  • Loading branch information
@HorizonNet @virajjasani
HorizonNet authored and virajjasani committed Dec 28, 2019
1 parent 0ba84d8 commit 703ee60
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 279 deletions.
39 changes: 20 additions & 19 deletions hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hadoop.hbase;

import java.io.File;
Expand Down Expand Up @@ -68,9 +67,17 @@ public static class Not implements ResourcePathFilter, FileNameFilter, ClassFilt
private FileNameFilter fileNameFilter;
private ClassFilter classFilter;

public Not(ResourcePathFilter resourcePathFilter){this.resourcePathFilter = resourcePathFilter;}
public Not(FileNameFilter fileNameFilter){this.fileNameFilter = fileNameFilter;}
public Not(ClassFilter classFilter){this.classFilter = classFilter;}
public Not(ResourcePathFilter resourcePathFilter) {
this.resourcePathFilter = resourcePathFilter;
}

public Not(FileNameFilter fileNameFilter) {
this.fileNameFilter = fileNameFilter;
}

public Not(ClassFilter classFilter) {
this.classFilter = classFilter;
}

@Override
public boolean isCandidatePath(String resourcePath, boolean isJar) {
Expand All @@ -90,7 +97,10 @@ public static class And implements ClassFilter, ResourcePathFilter {
ClassFilter[] classFilters;
ResourcePathFilter[] resourcePathFilters;

public And(ClassFilter...classFilters) { this.classFilters = classFilters; }
public And(ClassFilter...classFilters) {
this.classFilters = classFilters;
}

public And(ResourcePathFilter... resourcePathFilters) {
this.resourcePathFilters = resourcePathFilters;
}
Expand Down Expand Up @@ -120,10 +130,6 @@ public ClassFinder(ClassLoader classLoader) {
this(null, null, null, classLoader);
}

public ClassFinder() {
this(ClassLoader.getSystemClassLoader());
}

public ClassFinder(ResourcePathFilter resourcePathFilter, FileNameFilter fileNameFilter,
ClassFilter classFilter) {
this(resourcePathFilter, fileNameFilter, classFilter, ClassLoader.getSystemClassLoader());
Expand Down Expand Up @@ -193,7 +199,7 @@ public Set<Class<?>> findClasses(String packageName, boolean proceedOnExceptions
private Set<Class<?>> findClassesFromJar(String jarFileName,
String packageName, boolean proceedOnExceptions)
throws IOException, ClassNotFoundException, LinkageError {
JarInputStream jarFile = null;
JarInputStream jarFile;
try {
jarFile = new JarInputStream(new FileInputStream(jarFileName));
} catch (IOException ioEx) {
Expand All @@ -202,7 +208,7 @@ private Set<Class<?>> findClassesFromJar(String jarFileName,
}

Set<Class<?>> classes = new HashSet<>();
JarEntry entry = null;
JarEntry entry;
try {
while (true) {
try {
Expand Down Expand Up @@ -285,16 +291,11 @@ private Class<?> makeClass(String className, boolean proceedOnExceptions)
Class<?> c = Class.forName(className, false, classLoader);
boolean isCandidateClass = null == classFilter || classFilter.isCandidateClass(c);
return isCandidateClass ? c : null;
} catch (NoClassDefFoundError|ClassNotFoundException classNotFoundEx) {
if (!proceedOnExceptions) {
throw classNotFoundEx;
}
LOG.debug("Failed to instantiate or check " + className + ": " + classNotFoundEx);
} catch (LinkageError linkageEx) {
} catch (ClassNotFoundException | LinkageError exception) {
if (!proceedOnExceptions) {
throw linkageEx;
throw exception;
}
LOG.debug("Failed to instantiate or check " + className + ": " + linkageEx);
LOG.debug("Failed to instantiate or check " + className + ": " + exception);
}
return null;
}
Expand Down
59 changes: 34 additions & 25 deletions hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,29 +37,34 @@
/**
* Common helpers for testing HBase that do not depend on specific server/etc. things.
* @see org.apache.hadoop.hbase.HBaseCommonTestingUtility
*
*/
@InterfaceAudience.Public
public class HBaseCommonTestingUtility {
protected static final Logger LOG = LoggerFactory.getLogger(HBaseCommonTestingUtility.class);

/** Compression algorithms to use in parameterized JUnit 4 tests */
/**
* Compression algorithms to use in parameterized JUnit 4 tests
*/
public static final List<Object[]> COMPRESSION_ALGORITHMS_PARAMETERIZED =
Arrays.asList(new Object[][] {
{ Compression.Algorithm.NONE },
{ Compression.Algorithm.GZ }
});

/** This is for unit tests parameterized with a two booleans. */
/**
* This is for unit tests parameterized with a two booleans.
*/
public static final List<Object[]> BOOLEAN_PARAMETERIZED =
Arrays.asList(new Object[][] {
{false},
{true}
});

/** Compression algorithms to use in testing */
/**
* Compression algorithms to use in testing
*/
public static final Compression.Algorithm[] COMPRESSION_ALGORITHMS = {
Compression.Algorithm.NONE, Compression.Algorithm.GZ
Compression.Algorithm.NONE, Compression.Algorithm.GZ
};

protected Configuration conf;
Expand Down Expand Up @@ -98,9 +103,8 @@ public Configuration getConfiguration() {
private File dataTestDir = null;

/**
* @return Where to write test data on local filesystem, specific to
* the test. Useful for tests that do not use a cluster.
* Creates it if it does not exist already.
* @return Where to write test data on local filesystem, specific to the test. Useful for tests
* that do not use a cluster. Creates it if it does not exist already.
*/
public Path getDataTestDir() {
if (this.dataTestDir == null) {
Expand All @@ -110,10 +114,9 @@ public Path getDataTestDir() {
}

/**
* @param subdirName
* @return Path to a subdirectory named <code>subdirName</code> under
* {@link #getDataTestDir()}.
* Does *NOT* create it if it does not exist.
* @param subdirName the name of the subdirectory in the test data directory
* @return Path to a subdirectory named {code subdirName} under
* {@link #getDataTestDir()}. Does *NOT* create it if it does not exist.
*/
public Path getDataTestDir(final String subdirName) {
return new Path(getDataTestDir(), subdirName);
Expand All @@ -134,7 +137,10 @@ protected Path setupDataTestDir() {
this.dataTestDir = new File(testPath.toString()).getAbsoluteFile();
// Set this property so if mapreduce jobs run, they will use this as their home dir.
System.setProperty("test.build.dir", this.dataTestDir.toString());
if (deleteOnExit()) this.dataTestDir.deleteOnExit();

if (deleteOnExit()) {
this.dataTestDir.deleteOnExit();
}

createSubDir("hbase.local.dir", testPath, "hbase-local-dir");

Expand All @@ -154,11 +160,14 @@ public UUID getRandomUUID() {
ThreadLocalRandom.current().nextLong());
}


protected void createSubDir(String propertyName, Path parent, String subDirName) {
Path newPath = new Path(parent, subDirName);
File newDir = new File(newPath.toString()).getAbsoluteFile();
if (deleteOnExit()) newDir.deleteOnExit();

if (deleteOnExit()) {
newDir.deleteOnExit();
}

conf.set(propertyName, newDir.getAbsolutePath());
}

Expand All @@ -173,9 +182,8 @@ boolean deleteOnExit() {

/**
* @return True if we removed the test dirs
* @throws IOException
*/
public boolean cleanupTestDir() throws IOException {
public boolean cleanupTestDir() {
if (deleteDir(this.dataTestDir)) {
this.dataTestDir = null;
return true;
Expand All @@ -186,9 +194,8 @@ public boolean cleanupTestDir() throws IOException {
/**
* @param subdir Test subdir name.
* @return True if we removed the test dir
* @throws IOException
*/
boolean cleanupTestDir(final String subdir) throws IOException {
boolean cleanupTestDir(final String subdir) {
if (this.dataTestDir == null) {
return false;
}
Expand All @@ -197,9 +204,9 @@ boolean cleanupTestDir(final String subdir) throws IOException {

/**
* @return Where to write test data on local filesystem; usually
* {@link #DEFAULT_BASE_TEST_DIRECTORY}
* Should not be used by the unit tests, hence its's private.
* Unit test will use a subdirectory of this directory.
* {@link #DEFAULT_BASE_TEST_DIRECTORY}
* Should not be used by the unit tests, hence its's private.
* Unit test will use a subdirectory of this directory.
* @see #setupDataTestDir()
*/
private Path getBaseTestDir() {
Expand All @@ -212,17 +219,19 @@ private Path getBaseTestDir() {
/**
* @param dir Directory to delete
* @return True if we deleted it.
* @throws IOException
*/
boolean deleteDir(final File dir) throws IOException {
boolean deleteDir(final File dir) {
if (dir == null || !dir.exists()) {
return true;
}
int ntries = 0;
do {
ntries += 1;
try {
if (deleteOnExit()) FileUtils.deleteDirectory(dir);
if (deleteOnExit()) {
FileUtils.deleteDirectory(dir);
}

return true;
} catch (IOException ex) {
LOG.warn("Failed to delete " + dir.getAbsolutePath());
Expand Down
22 changes: 14 additions & 8 deletions hbase-common/src/test/java/org/apache/hadoop/hbase/ResourceChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hadoop.hbase;

import java.util.ArrayList;
Expand Down Expand Up @@ -47,7 +46,6 @@ public ResourceChecker(final String tagLine) {
this.tagLine = tagLine;
}


/**
* Class to implement for each type of resource.
*/
Expand Down Expand Up @@ -84,21 +82,22 @@ public String getName() {

/**
* The value for the resource.
* @param phase
* @param phase the {@link Phase} to get the value for
*/
abstract public int getVal(Phase phase);

/*
* Retrieves List of Strings which would be logged in logEndings()
*/
public List<String> getStringsToLog() { return null; }
public List<String> getStringsToLog() {
return null;
}
}

private List<ResourceAnalyzer> ras = new ArrayList<>();
private int[] initialValues;
private int[] endingValues;


private void fillInit() {
initialValues = new int[ras.size()];
fill(Phase.INITIAL, initialValues);
Expand Down Expand Up @@ -142,7 +141,11 @@ private void logInit() {
StringBuilder sb = new StringBuilder();
for (ResourceAnalyzer ra : ras) {
int cur = initialValues[i++];
if (sb.length() > 0) sb.append(", ");

if (sb.length() > 0) {
sb.append(", ");
}

sb.append(ra.getName()).append("=").append(cur);
}
LOG.info("before: " + tagLine + " " + sb);
Expand All @@ -157,7 +160,11 @@ private void logEndings() {
for (ResourceAnalyzer ra : ras) {
int curP = initialValues[i];
int curN = endingValues[i++];
if (sb.length() > 0) sb.append(", ");

if (sb.length() > 0) {
sb.append(", ");
}

sb.append(ra.getName()).append("=").append(curN).append(" (was ").append(curP).append(")");
if (curN > curP) {
List<String> strings = ra.getStringsToLog();
Expand All @@ -172,7 +179,6 @@ private void logEndings() {
LOG.info("after: " + tagLine + " " + sb);
}


/**
* To be called as the beginning of a test method:
* - measure the resources
Expand Down
21 changes: 9 additions & 12 deletions hbase-common/src/test/java/org/apache/hadoop/hbase/TestCellUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@

@Category({MiscTests.class, SmallTests.class})
public class TestCellUtil {

@ClassRule
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestCellUtil.class);
Expand Down Expand Up @@ -78,7 +77,7 @@ public Cell current() {
}

@Override
public boolean advance() throws IOException {
public boolean advance() {
if (this.count < cellsCount) {
this.current = new TestCell(this.count);
this.count++;
Expand Down Expand Up @@ -219,28 +218,28 @@ public long heapSize() {
*/
@Test
public void testCreateCellScannerOverflow() throws IOException {
consume(doCreateCellScanner(1, 1), 1 * 1);
consume(doCreateCellScanner(3, 0), 3 * 0);
consume(doCreateCellScanner(1, 1), 1);
consume(doCreateCellScanner(3, 0), 0);
consume(doCreateCellScanner(3, 3), 3 * 3);
consume(doCreateCellScanner(0, 1), 0 * 1);
consume(doCreateCellScanner(0, 1), 0);
// Do big number. See HBASE-11813 for why.
final int hundredK = 100000;
consume(doCreateCellScanner(hundredK, 0), hundredK * 0);
consume(doCreateCellScanner(hundredK, 0), 0);
consume(doCreateCellArray(1), 1);
consume(doCreateCellArray(0), 0);
consume(doCreateCellArray(3), 3);
List<CellScannable> cells = new ArrayList<>(hundredK);
for (int i = 0; i < hundredK; i++) {
cells.add(new TestCellScannable(1));
}
consume(CellUtil.createCellScanner(cells), hundredK * 1);
consume(CellUtil.createCellScanner(cells), hundredK);
NavigableMap<byte [], List<Cell>> m = new TreeMap<>(Bytes.BYTES_COMPARATOR);
List<Cell> cellArray = new ArrayList<>(hundredK);
for (int i = 0; i < hundredK; i++) {
cellArray.add(new TestCell(i));
}
m.put(new byte [] {'f'}, cellArray);
consume(CellUtil.createCellScanner(m), hundredK * 1);
consume(CellUtil.createCellScanner(m), hundredK);
}

private CellScanner doCreateCellArray(final int itemsPerList) {
Expand All @@ -251,8 +250,7 @@ private CellScanner doCreateCellArray(final int itemsPerList) {
return CellUtil.createCellScanner(cells);
}

private CellScanner doCreateCellScanner(final int listsCount, final int itemsPerList)
throws IOException {
private CellScanner doCreateCellScanner(final int listsCount, final int itemsPerList) {
List<CellScannable> cells = new ArrayList<>(listsCount);
for (int i = 0; i < listsCount; i++) {
CellScannable cs = new CellScannable() {
Expand Down Expand Up @@ -554,11 +552,10 @@ public void testWriteCell() throws IOException {

// Workaround for jdk 11 - reflective access to interface default methods for testGetType
private abstract class CellForMockito implements Cell {

}

@Test
public void testGetType() throws IOException {
public void testGetType() {
CellForMockito c = Mockito.mock(CellForMockito.class);
Mockito.when(c.getType()).thenCallRealMethod();
for (CellForMockito.Type type : CellForMockito.Type.values()) {
Expand Down
Loading

0 comments on commit 703ee60

Please sign in to comment.