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.

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

--filter option implemented. #647

Merged
merged 23 commits into from
Jun 5, 2013
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
196c4ed
--filter option implemented.
noel-yap Feb 28, 2013
0a9a389
Changes due to suggestions from @kcooney.
noel-yap Mar 1, 2013
318adee
JavaDoc added.
noel-yap Mar 5, 2013
5edfb44
Merge branch 'master' into filter-option
noel-yap Mar 7, 2013
3433608
Modification in response to code review from @kcooney and @dsaff.
noel-yap Mar 14, 2013
15c6048
More tests added.
noel-yap Mar 15, 2013
74cab2d
crlf fixed.
noel-yap Mar 18, 2013
5820172
API simplified. FilterFactory.parseArgs() removed. FilterFactoryParam…
noel-yap Mar 20, 2013
9d9899b
Responses to code review from @dsaff.
noel-yap Mar 21, 2013
aa7c75d
Test added.
noel-yap Mar 21, 2013
40b8761
JavaDoc added.
noel-yap Mar 21, 2013
b01900f
FilterFactoryFactory renamed to FilterFactories and its methods made …
noel-yap Mar 27, 2013
114025f
public keyword removed from methods of interface.
noel-yap Mar 27, 2013
b1cf4b5
Merge branch 'master' into filter-option
noel-yap Mar 27, 2013
5619304
Command parsing errors internalized such that JUnitCore doesn't have …
noel-yap Apr 1, 2013
8265296
Changes in response to code review from @kcooney.
noel-yap Apr 29, 2013
b0c0aaf
Further changes due to code review from @kcooney.
noel-yap Apr 30, 2013
b5084eb
JavaDoc fixed.
noel-yap Apr 30, 2013
c85a267
Changes due to code review from @dsaff.
noel-yap May 2, 2013
ae1fc25
Javadoc removed. Comments to be created by @kcooney.
noel-yap May 2, 2013
cfaceb6
Changes due to code review from @dsaff.
noel-yap May 6, 2013
fa5df4e
Responses to code review from @dsaff.
noel-yap May 27, 2013
b026c08
Responses to code review from @dsaff.
noel-yap May 27, 2013
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.Set;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import org.junit.runner.Description;
import org.junit.runner.manipulation.Filter;
import org.junit.runner.manipulation.NoTestsRemainException;
Expand Down Expand Up @@ -154,7 +155,7 @@ public static CategoryFilter categoryFilter(boolean matchAnyInclusions, Set<Clas
return new CategoryFilter(matchAnyInclusions, inclusions, matchAnyExclusions, exclusions);
}

private CategoryFilter(boolean matchAnyIncludes, Set<Class<?>> includes,
protected CategoryFilter(boolean matchAnyIncludes, Set<Class<?>> includes,
boolean matchAnyExcludes, Set<Class<?>> excludes) {
fIncludedAny= matchAnyIncludes;
fExcludedAny= matchAnyExcludes;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package org.junit.experimental.categories;

import java.util.ArrayList;
import java.util.List;

import org.junit.internal.Classes;
import org.junit.runner.FilterFactory;
import org.junit.runner.FilterFactoryParams;
import org.junit.runner.manipulation.Filter;

/**
* Implementation of FilterFactory for Category filtering.
*/
public abstract class CategoryFilterFactory implements FilterFactory {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be package-scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

/**
* Creates a {@link org.junit.experimental.categories.Categories.CategoryFilter} given a
* ${FilterFactoryParams} argument.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is ${} here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

*
* @param params Parameters needed to create the {@link Filter}
* @throws FilterNotCreatedException
*/
@Override
public Filter createFilter(FilterFactoryParams params) throws FilterNotCreatedException {
try {
return createFilter(parseCategories(params.getArgs()));
} catch (ClassNotFoundException e) {
throw new FilterNotCreatedException(e);
}
}

/**
* Creates a {@link org.junit.experimental.categories.Categories.CategoryFilter} given an array of classes used as
* {@link Category} values.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking, it takes Classes, which may be values of @Category annotations. (I think the wording used on ExcludeCategories is good.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

*
* @param categories Category classes.
*/
protected abstract Filter createFilter(Class<?>... categories);

private Class<?>[] parseCategories(String categories) throws ClassNotFoundException {
List<Class<?>> categoryClasses = new ArrayList<Class<?>>();

for (String category : categories.split(",")) {
Class<?> categoryClass = Classes.getClass(category);

categoryClasses.add(categoryClass);
}

return categoryClasses.toArray(new Class[]{});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.junit.experimental.categories;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

import org.junit.runner.manipulation.Filter;

import static org.junit.experimental.categories.Categories.CategoryFilter;

/**
* {@link org.junit.runner.FilterFactory} to exclude categories.
*
* The {@link Filter} that is created will filter out tests that are categorized with any of the
* given categories.
*
* Usage from command line:
* <code>
* --filter=org.junit.experimental.categories.ExcludeCategories=pkg.of.Cat1,pkg.of.Cat2
* </code>
*
* Usage from API:
* <code>
* new ExcludeCategories().createFilter(Cat1.class, Cat2.class);
* </code>
*/
public final class ExcludeCategories extends CategoryFilterFactory {
/**
* Creates an {@link ExcludesAny} {@link CategoryFilter} given an array of classes used as
* {@link Category} values.
*
* @param categories Category classes.
* @return
*/
@Override
protected Filter createFilter(Class<?>... categories) {
return new ExcludesAny(categories);
}

private static class ExcludesAny extends CategoryFilter {
public ExcludesAny(Class<?>... categories) {
this(new HashSet<Class<?>>(Arrays.asList(categories)));
}

public ExcludesAny(Set<Class<?>> categories) {
super(true, null, true, categories);
}

@Override
public String describe() {
return "excludes " + super.describe();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.junit.experimental.categories;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

import org.junit.runner.manipulation.Filter;

import static org.junit.experimental.categories.Categories.CategoryFilter;

/**
* {@link org.junit.runner.FilterFactory} to include categories.
*
* The {@link Filter} that is created will filter out tests that are categorized with any of the
* given categories.
*
* Usage from command line:
* <code>
* --filter=org.junit.experimental.categories.IncludeCategories=pkg.of.Cat1,pkg.of.Cat2
* </code>
*
* Usage from API:
* <code>
* new IncludeCategories().createFilter(Cat1.class, Cat2.class);
* </code>
*/
public final class IncludeCategories extends CategoryFilterFactory {
/**
* Creates an {@link IncludesAny} {@link CategoryFilter} given an array of classes used as
* {@link Category} values.
*
* @param categories Category classes.
* @return
*/
@Override
protected Filter createFilter(Class<?>... categories) {
return new IncludesAny(categories);
}

private static class IncludesAny extends CategoryFilter {
public IncludesAny(Class<?>... categories) {
this(new HashSet<Class<?>>(Arrays.asList(categories)));
}

public IncludesAny(Set<Class<?>> categories) {
super(true, categories, true, null);
}

@Override
public String describe() {
return "includes " + super.describe();
}
}
}
18 changes: 18 additions & 0 deletions src/main/java/org/junit/internal/Classes.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.junit.internal;

import static java.lang.Thread.currentThread;

/**
* Miscellaneous functions dealing with classes.
*/
public class Classes {
/**
* Returns Class.forName for {@code className} using the current thread's class loader.
*
* @param className Name of the class.
* @throws ClassNotFoundException
*/
public static Class<?> getClass(String className) throws ClassNotFoundException {
return Class.forName(className, true, currentThread().getContextClassLoader());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ private List<Throwable> getCauses(Throwable cause) {

private Description describeCause(Throwable child) {
return Description.createTestDescription(fTestClass,
"initializationError");
"initializationError: " + child);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces in descriptions give some frameworks fits. The throwable should be available in the stacktrace, can we leave it out of the description?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

}

private void runCause(Throwable child, RunNotifier notifier) {
Expand All @@ -62,4 +62,4 @@ private void runCause(Throwable child, RunNotifier notifier) {
notifier.fireTestFailure(new Failure(description, child));
notifier.fireTestFinished(description);
}
}
}
94 changes: 94 additions & 0 deletions src/main/java/org/junit/runner/FilterFactories.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package org.junit.runner;

import org.junit.internal.Classes;
import org.junit.runner.manipulation.Filter;

import static org.junit.runner.FilterFactory.FilterNotCreatedException;

/**
* Utility class whose methods create a {@link FilterFactory}.
*/
public class FilterFactories {
/**
* Creates a {@link Filter}.
*
* A filter specification is of the form "package.of.FilterFactory=args-to-filter-factory" or
* "package.of.FilterFactory".
*
* @param filterSpec The filter specification
* @throws FilterFactoryNotCreatedException
* @throws FilterNotCreatedException
*/
public static Filter createFilterFromFilterSpec(Description description, String filterSpec)
throws FilterFactoryNotCreatedException {

if (filterSpec.contains("=")) {
String[] tuple = filterSpec.split("=", 2);

return createFilter(tuple[0], new FilterFactoryParams(description, tuple[1]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kcooney, is this how you were hoping to add descriptions, or would you rather Noel hold off and let you add descriptions when this is done?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsaff This isn't what I had in mind. I suggest removing the Description parameter from FilterFactoryParams for now. I can add it later; let's not add more things to go back and forth about in this change (it's actually pretty close iMHO)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

} else {
return createFilter(filterSpec, new FilterFactoryParams(description));
}
}

/**
* Creates a {@link Filter}.
*
* @param filterFactoryFqcn The fully qualified class name of the {@link FilterFactory}
* @param params The arguments to the {@link FilterFactory}
* @throws FilterNotCreatedException
* @throws FilterFactoryNotCreatedException
*/
public static Filter createFilter(String filterFactoryFqcn, FilterFactoryParams params)
throws FilterFactoryNotCreatedException {
FilterFactory filterFactory = createFilterFactory(filterFactoryFqcn);

return filterFactory.createFilter(params);
}

/**
* Creates a {@link Filter}.
*
* @param filterFactoryClass The class of the {@link FilterFactory}
* @param params The arguments to the {@link FilterFactory}
* @throws FilterNotCreatedException
* @throws FilterFactoryNotCreatedException
*
*/
public static Filter createFilter(Class<? extends FilterFactory> filterFactoryClass, FilterFactoryParams params)
throws FilterFactoryNotCreatedException {
FilterFactory filterFactory = createFilterFactory(filterFactoryClass);

return filterFactory.createFilter(params);
}

static FilterFactory createFilterFactory(String filterFactoryFqcn) throws FilterFactoryNotCreatedException {
Class<? extends FilterFactory> filterFactoryClass;

try {
filterFactoryClass = Classes.getClass(filterFactoryFqcn).asSubclass(FilterFactory.class);
} catch (Exception e) {
throw new FilterFactoryNotCreatedException(e);
}

return createFilterFactory(filterFactoryClass);
}

static FilterFactory createFilterFactory(Class<? extends FilterFactory> filterFactoryClass)
throws FilterFactoryNotCreatedException {
try {
return filterFactoryClass.getConstructor().newInstance();
} catch (Exception e) {
throw new FilterFactoryNotCreatedException(e);
}
}

/**
* Exception thrown if the {@link FilterFactory} cannot be created.
*/
public static class FilterFactoryNotCreatedException extends ClassNotFoundException {
public FilterFactoryNotCreatedException(Exception exception) {
super(exception.getMessage(), exception);
}
}
}
25 changes: 25 additions & 0 deletions src/main/java/org/junit/runner/FilterFactory.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.junit.runner;

import org.junit.runner.manipulation.Filter;

/**
* Extend this class to create a factory that creates {@link Filter}.
*/
public interface FilterFactory {
/**
* Creates a {@link Filter} given a ${FilterFactoryParams} argument.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ -> @link

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

*
* @param params Parameters needed to create the {@link Filter}
* @throws FilterNotCreatedException
*/
Filter createFilter(FilterFactoryParams params) throws FilterNotCreatedException;

/**
* Exception thrown if the {@link Filter} cannot be created.
*/
static class FilterNotCreatedException extends FilterFactories.FilterFactoryNotCreatedException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems odd that FilterNotCreatedException is a special case of `FitlerFactoryNotCreatedException'. Also, shouldn't this be public? It seems odd to have a method in a public interface throw a package-scope exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hierarchy reversed.

public FilterNotCreatedException(Exception exception) {
super(exception);
}
}
}
43 changes: 43 additions & 0 deletions src/main/java/org/junit/runner/FilterFactoryParams.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package org.junit.runner;

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd be better off without any of the javadoc in this class. Better would be a brief class-level comment about why this exists (especially because it's taken @kcooney several times before I got my head around why this is a good idea).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kcooney , preference for the class-level comment?

* Parameters to a {@link FilterFactory}.
*/
public final class FilterFactoryParams {
private final Description description;
private final String args;

/**
* Constructs a {@link FilterFactoryParams}.
*
* @param description {@link Description}
*/
public FilterFactoryParams(Description description) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should javadoc where the FilterFactory can expect the Description and args to have come from.

this(description, null);
}

/**
* Constructs a {@link FilterFactoryParams}.
*
* @param description {@link Description}
* @param args Arguments
*/
public FilterFactoryParams(Description description, String args) {
this.description = description;
this.args = args;
}

/**
* Returns the {@link Description}.
*/
public Description getDescription() {
return description;
}

/**
* Returns the arguments.
*/
public String getArgs() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this makes more sense as getArgs or getArg?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it depends upon how one looks at it. The field can contain zero or more arguments depending upon how its user decides to parse it. @kcooney , opinions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@noel-yap @dsaff I think getArgs makes sense

return args;
}
}
Loading