Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-23621 Reduced the number of Checkstyle violations in tests of hbase-common #1030

Merged
merged 3 commits into from
Jan 27, 2020
Merged

HBASE-23621 Reduced the number of Checkstyle violations in tests of hbase-common #1030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f66a2d9
HBASE-23621 Reduced the number of Checkstyle violations in tests of h…
HorizonNet Dec 27, 2019
1a4cde6
HBASE-23621 Fixed review comment
HorizonNet Jan 16, 2020
a7b3985
Merge branch 'branch-1' into HBASE-23621_branch-1
HorizonNet Jan 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 28 additions & 27 deletions hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java
View file
Open in desktop
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 @@ -53,24 +52,32 @@ public class ClassFinder {

public interface ResourcePathFilter {
boolean isCandidatePath(String resourcePath, boolean isJar);
};
}

public interface FileNameFilter {
boolean isCandidateFile(String fileName, String absFilePath);
};
}

public interface ClassFilter {
boolean isCandidateClass(Class<?> c);
};
}

public static class Not implements ResourcePathFilter, FileNameFilter, ClassFilter {
private ResourcePathFilter resourcePathFilter;
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 @@ -180,7 +186,7 @@ public Set<Class<?>> findClasses(String packageName, boolean proceedOnExceptions
}
}

Set<Class<?>> classes = new HashSet<Class<?>>();
Set<Class<?>> classes = new HashSet<>();
for (File directory : dirs) {
classes.addAll(findClassesFromFiles(directory, packageName, proceedOnExceptions));
}
Expand All @@ -193,16 +199,16 @@ 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) {
LOG.warn("Failed to look for classes in " + jarFileName + ": " + ioEx);
throw ioEx;
}

Set<Class<?>> classes = new HashSet<Class<?>>();
JarEntry entry = null;
Set<Class<?>> classes = new HashSet<>();
JarEntry entry;
try {
while (true) {
try {
Expand Down Expand Up @@ -248,7 +254,7 @@ private Set<Class<?>> findClassesFromJar(String jarFileName,

private Set<Class<?>> findClassesFromFiles(File baseDirectory, String packageName,
boolean proceedOnExceptions) throws ClassNotFoundException, LinkageError {
Set<Class<?>> classes = new HashSet<Class<?>>();
Set<Class<?>> classes = new HashSet<>();
if (!baseDirectory.exists()) {
LOG.warn("Failed to find " + baseDirectory.getAbsolutePath());
return classes;
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 (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 All @@ -313,5 +314,5 @@ public boolean accept(File file) {
&& (null == nameFilter
|| nameFilter.isCandidateFile(file.getName(), file.getAbsolutePath())));
}
};
};
}
}
43 changes: 24 additions & 19 deletions hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseCommonTestingUtility.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,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 @@ -88,10 +87,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 @@ -115,7 +113,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 @@ -125,7 +126,11 @@ protected Path setupDataTestDir() {
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 @@ -140,9 +145,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 @@ -153,9 +157,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 @@ -164,9 +167,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 @@ -185,17 +188,19 @@ public Path getRandomDir() {
/**
* @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
Open in desktop
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 org.apache.commons.logging.Log;
Expand Down Expand Up @@ -46,7 +45,6 @@ public ResourceChecker(final String tagLine) {
this.tagLine = tagLine;
}


/**
* Class to implement for each type of resource.
*/
Expand Down Expand Up @@ -83,21 +81,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<ResourceAnalyzer>();
private int[] initialValues;
private int[] endingValues;


private void fillInit() {
initialValues = new int[ras.size()];
fill(Phase.INITIAL, initialValues);
Expand Down Expand Up @@ -141,7 +140,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 @@ -156,7 +159,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 @@ -171,7 +178,6 @@ private void logEndings() {
LOG.info("after: " + tagLine + " " + sb);
}


/**
* To be called as the beginning of a test method:
* - measure the resources
Expand Down
Loading