-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Support for Multiple Include/Exclude Categories #340
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
Changes from 3 commits
a434eee
83ac907
56ed0d7
5fc05a0
942e89f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,34 +69,53 @@ public class Categories extends Suite { | |
|
||
@Retention(RetentionPolicy.RUNTIME) | ||
public @interface IncludeCategory { | ||
public Class<?> value(); | ||
public Class<?>[] value(); | ||
} | ||
|
||
@Retention(RetentionPolicy.RUNTIME) | ||
public @interface ExcludeCategory { | ||
public Class<?> value(); | ||
public Class<?>[] value(); | ||
} | ||
|
||
public static class CategoryFilter extends Filter { | ||
public static CategoryFilter include(Class<?> categoryType) { | ||
return new CategoryFilter(categoryType, null); | ||
public static CategoryFilter include(Class<?>... categoryTypes) { | ||
return new CategoryFilter(categoryTypes, null); | ||
} | ||
|
||
public static CategoryFilter exclude(Class<?>... categoryTypes) { | ||
return new CategoryFilter(null, categoryTypes); | ||
} | ||
|
||
private final Class<?> fIncluded; | ||
|
||
private final Class<?> fExcluded; | ||
private final Class<?>[] fIncluded; | ||
|
||
public CategoryFilter(Class<?> includedCategory, | ||
Class<?> excludedCategory) { | ||
fIncluded= includedCategory; | ||
fExcluded= excludedCategory; | ||
private final Class<?>[] fExcluded; | ||
|
||
public CategoryFilter(Class<?>[] includedCategories, | ||
Class<?>[] excludedCategories) { | ||
fIncluded= includedCategories; | ||
fExcluded= excludedCategories; | ||
} | ||
|
||
@Override | ||
public String describe() { | ||
return "category " + fIncluded; | ||
return ((fIncluded == null || fIncluded.length == 1) ? "category ":"categories ") + join(", ", fIncluded); | ||
} | ||
|
||
private String join(String seperator, Class<?>... values) | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please move brace to previous line |
||
if(values == null || values.length == 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Space after "if" |
||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove braces for one-line clauses |
||
return ""; | ||
} | ||
StringBuilder sb = new StringBuilder(values[0].toString()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JUnit style is StringBuilder sb= new... (no space before the =). I hate it, but it's what we have now. |
||
for(int i = 1; i < values.length; i++) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for (Class<?> each : values) |
||
{ | ||
sb.append(seperator).append(values[i].toString()); | ||
} | ||
return sb.toString(); | ||
} | ||
|
||
|
||
@Override | ||
public boolean shouldRun(Description description) { | ||
if (hasCorrectCategoryAnnotation(description)) | ||
|
@@ -112,10 +131,28 @@ private boolean hasCorrectCategoryAnnotation(Description description) { | |
if (categories.isEmpty()) | ||
return fIncluded == null; | ||
for (Class<?> each : categories) | ||
if (fExcluded != null && fExcluded.isAssignableFrom(each)) | ||
if (shouldExclude(each)) | ||
return false; | ||
for (Class<?> each : categories) | ||
if (fIncluded == null || fIncluded.isAssignableFrom(each)) | ||
if (shouldInclude(each)) | ||
return true; | ||
return false; | ||
} | ||
|
||
private boolean shouldInclude(Class<?> category) { | ||
if (fIncluded == null) | ||
return true; | ||
for(Class<?> includeCat : fIncluded) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you combine this loop with the one below into a single helper method? |
||
if (includeCat.isAssignableFrom(category)) | ||
return true; | ||
return false; | ||
} | ||
|
||
private boolean shouldExclude(Class<?> category) { | ||
if (fExcluded == null) | ||
return false; | ||
for(Class<?> each : fExcluded) | ||
if(each.isAssignableFrom(category)) | ||
return true; | ||
return false; | ||
} | ||
|
@@ -148,20 +185,20 @@ public Categories(Class<?> klass, RunnerBuilder builder) | |
throws InitializationError { | ||
super(klass, builder); | ||
try { | ||
filter(new CategoryFilter(getIncludedCategory(klass), | ||
getExcludedCategory(klass))); | ||
filter(new CategoryFilter(getIncludedCategories(klass), | ||
getExcludedCategories(klass))); | ||
} catch (NoTestsRemainException e) { | ||
throw new InitializationError(e); | ||
} | ||
assertNoCategorizedDescendentsOfUncategorizeableParents(getDescription()); | ||
} | ||
|
||
private Class<?> getIncludedCategory(Class<?> klass) { | ||
IncludeCategory annotation= klass.getAnnotation(IncludeCategory.class); | ||
private Class<?>[] getIncludedCategories(Class<?> klass) { | ||
IncludeCategory annotation = klass.getAnnotation(IncludeCategory.class); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no space before = |
||
return annotation == null ? null : annotation.value(); | ||
} | ||
|
||
private Class<?> getExcludedCategory(Class<?> klass) { | ||
private Class<?>[] getExcludedCategories(Class<?> klass) { | ||
ExcludeCategory annotation= klass.getAnnotation(ExcludeCategory.class); | ||
return annotation == null ? null : annotation.value(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,10 @@ public interface FastTests { | |
public interface SlowTests { | ||
// category marker | ||
} | ||
|
||
public interface ReallySlowTests { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you write tests that exercise the multiple-category functionality? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was already a test that exercised multi-category exclude. I renamed it to make this more evident and also added another that tests multi-category includes. see testCountWithMultipleExcludeFilter and testCountWithMultipleIncludeFilter |
||
// category marker | ||
} | ||
|
||
public static class A { | ||
@Test | ||
|
@@ -133,6 +137,15 @@ public void testCountWithExplicitFilter() throws Throwable { | |
assertTrue(result.wasSuccessful()); | ||
assertEquals(2, result.getRunCount()); | ||
} | ||
|
||
@Test | ||
public void testCountWithExplicitExcludeFilter() throws Throwable { | ||
CategoryFilter exclude = CategoryFilter.exclude(SlowTests.class, FastTests.class); | ||
Request baseRequest= Request.aClass(OneOfEach.class); | ||
Result result= new JUnitCore().run(baseRequest.filterWith(exclude)); | ||
assertTrue(result.wasSuccessful()); | ||
assertEquals(1, result.getRunCount()); | ||
} | ||
|
||
@Test | ||
public void categoryFilterLeavesOnlyMatchingMethods() | ||
|
@@ -143,7 +156,7 @@ public void categoryFilterLeavesOnlyMatchingMethods() | |
assertEquals(1, runner.testCount()); | ||
} | ||
|
||
public static class OneFastOneSlow { | ||
public static class OneOfEach { | ||
@Category(FastTests.class) | ||
@Test | ||
public void a() { | ||
|
@@ -155,14 +168,20 @@ public void a() { | |
public void b() { | ||
|
||
} | ||
|
||
@Category(ReallySlowTests.class) | ||
@Test | ||
public void c() { | ||
|
||
} | ||
} | ||
|
||
@Test | ||
public void categoryFilterRejectsIncompatibleCategory() | ||
throws InitializationError, NoTestsRemainException { | ||
CategoryFilter filter= CategoryFilter.include(SlowTests.class); | ||
BlockJUnit4ClassRunner runner= new BlockJUnit4ClassRunner( | ||
OneFastOneSlow.class); | ||
OneOfEach.class); | ||
filter.apply(runner); | ||
assertEquals(1, runner.testCount()); | ||
} | ||
|
@@ -194,6 +213,12 @@ public void describeACategoryFilter() { | |
assertEquals("category " + SlowTests.class, filter.describe()); | ||
} | ||
|
||
@Test | ||
public void describeAMultiCategoryFilter() { | ||
CategoryFilter filter= CategoryFilter.include(SlowTests.class, FastTests.class); | ||
assertEquals("categories " + SlowTests.class + ", " + FastTests.class, filter.describe()); | ||
} | ||
|
||
public static class OneThatIsBothFastAndSlow { | ||
@Category({FastTests.class, SlowTests.class}) | ||
@Test | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please join with previous line